Re: [PATCH 1/4] ceph: bound xattr value length in __build_xattrs()

[Viacheslav Dubeyko]

On Thu, 2026-06-04 at 14:08 -0400, Michael Bommarito wrote:
> __build_xattrs() decodes the MDS-supplied xattr blob one attribute at
> a
> time. For each attribute it reads a 32-bit name length, advances past
> the
> name bytes, reads a 32-bit value length, records the value pointer,
> and
> advances past the value bytes. The two length fields are read with
> ceph_decode_32_safe(), but the value bytes themselves are advanced
> over
> with a bare "p += len" and no ceph_decode_need() check that "len"
> bytes
> remain in the blob.
> 
> For every attribute except the last, the next iteration's
> ceph_decode_32_safe() on the following name length implicitly
> verifies
> that the previous value did not run past the blob end. The final
> attribute has no successor, so its decoded value length is never
> checked
> against the blob bounds. A malicious or compromised metadata server
> can
> set the last attribute's value length larger than the bytes actually
> present in the blob.
> 
> The blob is a dedicated kvmalloc() allocation sized to the wire
> length
> (ceph_buffer_new() in ceph_fill_inode()). __set_xattr() records the
> oversized length in xattr->val_len verbatim, and a later getxattr(2)
> runs
> memcpy(value, xattr->val, xattr->val_len) into a user-supplied
> buffer,
> copying bytes past the end of the allocation back to user space.
> 
> Impact: a malicious metadata server discloses adjacent kernel heap
> bytes
> to a local user via getxattr(2) on a CephFS file. Add the missing
> ceph_decode_need() so an out-of-bounds value length on the final
> attribute fails the decode and returns -EIO instead of being stored.
> 
> Fixes: 355da1eb7a1f ("ceph: inode operations")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> ---
>  fs/ceph/xattr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index e773be07f7674..d488bb8fc00ba 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -848,6 +848,7 @@ static int __build_xattrs(struct inode *inode)
>  			name = p;
>  			p += len;
>  			ceph_decode_32_safe(&p, end, len, bad);
> +			ceph_decode_need(&p, end, len, bad);
>  			val = p;
>  			p += len;
>  

Makes sense.

Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>

Thanks,
Slava.