Re: [PATCH 2/4] ceph: bound MDSCapAuth path and fs_name decode in handle_session()

[Viacheslav Dubeyko]

On Thu, 2026-06-04 at 14:08 -0400, Michael Bommarito wrote:
> handle_session() decodes the MDSCapAuth records carried by a
> CEPH_SESSION_OPEN message (msg_version >= 6). For each record the
> match.path and match.fs_name byte strings are read by first decoding
> a
> 32-bit length and then copying that many bytes with the bare
> ceph_decode_copy(). Unlike the surrounding fields, which all use the
> _safe decode variants, these two copies are not preceded by a
> ceph_decode_need() bounds check, and the enclosing MDSCapAuth and
> MDSCapMatch struct_len fields are skipped rather than enforced as an
> upper bound. A length larger than the bytes remaining in the message
> front makes ceph_decode_copy() read past the end of the front buffer.
> 
> The message front is a dedicated allocation (ceph_msg_new2() ->
> kvmalloc), so the over-read runs off that object. A malicious or
> compromised MDS can trigger this with the first post-connect message
> on
> mount, with no client-side user interaction; under KASAN it is
> reported
> as a slab-out-of-bounds read in handle_session().
> 
> Impact: a malicious MDS can force the kernel client to read up to 4
> GiB
> past the message front allocation during session setup, crashing the
> client (out-of-bounds read).
> 
> Switch both copies to ceph_decode_copy_safe(), which performs the
> ceph_decode_need() bounds check before the copy and branches to the
> existing bad label, matching the rest of the decoder and the error
> path
> that frees the partially decoded cap_auths array.
> 
> Fixes: 1d17de9534cb ("ceph: save cap_auths in MDS client when session
> is opened")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> ---
>  fs/ceph/mds_client.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ed17e0023705e..4f36ac73305dc 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -4318,7 +4318,9 @@ static void handle_session(struct
> ceph_mds_session *session,
>  					pr_err_client(cl, "No memory
> for path\n");
>  					goto fail;
>  				}
> -				ceph_decode_copy(&p,
> cap_auths[i].match.path, _len);
> +				ceph_decode_copy_safe(&p, end,
> +						     
> cap_auths[i].match.path,
> +						      _len, bad);
>  
>  				/* Remove the tailing '/' */
>  				while (_len &&
> cap_auths[i].match.path[_len - 1] == '/') {
> @@ -4335,7 +4337,9 @@ static void handle_session(struct
> ceph_mds_session *session,
>  					pr_err_client(cl, "No memory
> for fs_name\n");
>  					goto fail;
>  				}
> -				ceph_decode_copy(&p,
> cap_auths[i].match.fs_name, _len);
> +				ceph_decode_copy_safe(&p, end,
> +						     
> cap_auths[i].match.fs_name,
> +						      _len, bad);
>  			}
>  
>  			ceph_decode_8_safe(&p, end,
> cap_auths[i].match.root_squash, bad);

Looks good.

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

Thanks,
Slava.