Re: [PATCH] libceph: fix potential out-of-bounds read in decode_new_up_state_weight()

[Raphael Zimmer]

On 06.06.26 5:51 PM, Zhenhao Wan wrote:
> The new_state section of an incremental OSD map is validated and skipped
> using a byte count computed as
> 
> 	len *= sizeof(u32) + (struct_v >= 5 ? sizeof(u32) : sizeof(u8));
> 
> The multiplication is evaluated in size_t, but the result is stored back
> into the u32 "len", truncating it.  A malicious or corrupted incremental
> map can supply a new_state element count >= 0x20000000 (struct_v >= 5) so
> that len * 8 wraps modulo 2^32 to a small value.  The following
> ceph_decode_need() then validates far fewer bytes than the section
> actually occupies.
> 
> new_state is then reprocessed with the unchecked ceph_decode_32() and
> ceph_decode_8() helpers, which have no per-iteration bounds check and
> rely entirely on that truncated up-front validation.  This can lead to
> a kernel out-of-bounds read past "end".
> 
> Compute the byte count in u64 and bounds-check it against the remaining
> buffer before skipping, mirroring the size_t-typed length checks used
> elsewhere in this file (e.g. decode_crush_names(), decode_pg_mapping()).
> The osd index used for the osd_state[] write is already bounds-checked
> against map->max_osd, so this is an out-of-bounds read, not a write.
> 
> Fixes: 930c53286977 ("libceph: apply new_state before new_up_client on incrementals")
> Reported-by: Yuhao Jiang <danisjiang@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Zhenhao Wan <whi4ed0g@xxxxxxxxx>
> ---
>  net/ceph/osdmap.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 8b5b0587a0cf..dd3023fe821e 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -1842,6 +1842,7 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v,
>  	void *new_up_client;
>  	void *new_state;
>  	void *new_weight_end;
> +	u64 skip_len;
>  	u32 len;
>  	int ret;
>  	int i;
> @@ -1862,9 +1863,10 @@ static int decode_new_up_state_weight(void **p, void *end, u8 struct_v,
> 
>  	new_state = *p;
>  	ceph_decode_32_safe(p, end, len, e_inval);
> -	len *= sizeof(u32) + (struct_v >= 5 ? sizeof(u32) : sizeof(u8));
> -	ceph_decode_need(p, end, len, e_inval);
> -	*p += len;
> +	skip_len = (u64)len * (sizeof(u32) + (struct_v >= 5 ? sizeof(u32) : sizeof(u8)));
> +	if (skip_len > end - *p)
> +		goto e_inval;
> +	*p += skip_len;
> 
>  	/* new_weight */
>  	ceph_decode_32_safe(p, end, len, e_inval);
> 
> ---
> base-commit: dbe8d05c9750b107b10c15361aad40fbb350bedb
> change-id: 20260606-ceph-fix-final-16f4e1df5a5e
> 
> Best regards,
> --
> Zhenhao Wan <whi4ed0g@xxxxxxxxx>

Hi,
a patch for this issue has already been discussed on the ceph-devel
mailing list here:
https://lore.kernel.org/ceph-devel/b6c16cd9aa7bc31240a133d68cec03ea914f918a.camel@xxxxxxx/T/#t

Best regards,
Raphael