Re: [PATCH] ceph: bound copied dentry name length in NFS export get_name

[Viacheslav Dubeyko]

On Sat, 2026-07-11 at 11:07 -0400, Michael Bommarito wrote:
> ceph_get_name() copies the MDS-supplied name into the caller's
> NAME_MAX-sized buffer with memcpy(name, rinfo->dname, rinfo-
> >dname_len)
> and then writes name[rinfo->dname_len] = 0, without checking
> dname_len
> against NAME_MAX. A malicious or buggy MDS that returns a LOOKUPNAME
> reply
> with dname_len > NAME_MAX overflows the buffer. __get_snap_name()
> copies
> rde->name / rde->name_len the same unchecked way.
> 
> Impact: a malicious or compromised Ceph MDS overflows the NAME_MAX
> name
> buffer in a client's NFS-export get_name path, a slab out-of-bounds
> write
> reported by KASAN. Reachable when a CephFS mount is re-exported over
> NFS.
> 
> Add ceph_export_copy_name(), which rejects lengths above NAME_MAX
> with
> -ENAMETOOLONG before the copy, and use it in both ceph_get_name() and
> __get_snap_name().
> 
> Fixes: 19913b4eac4a ("ceph: add get_name() NFS export callback")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> ---
>  fs/ceph/export.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index b2f2af1046791..debb9634b9e3d 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -442,6 +442,16 @@ static struct dentry *ceph_fh_to_parent(struct
> super_block *sb,
>  	return dentry;
>  }
>  
> +static int ceph_export_copy_name(char *name, const char *src, u32
> len)
> +{
> +	if (len > NAME_MAX)
> +		return -ENAMETOOLONG;
> +
> +	memcpy(name, src, len);
> +	name[len] = '\0';
> +	return 0;
> +}
> +
>  static int __get_snap_name(struct dentry *parent, char *name,
>  			   struct dentry *child)
>  {
> @@ -513,9 +523,8 @@ static int __get_snap_name(struct dentry *parent,
> char *name,
>  			BUG_ON(!rde->inode.in);
>  			if (ceph_snap(inode) ==
>  			    le64_to_cpu(rde->inode.in->snapid)) {
> -				memcpy(name, rde->name, rde-
> >name_len);
> -				name[rde->name_len] = '\0';
> -				err = 0;
> +				err = ceph_export_copy_name(name,
> rde->name,
> +							    rde-
> >name_len);
>  				goto out;
>  			}
>  		}
> @@ -580,8 +589,8 @@ static int ceph_get_name(struct dentry *parent,
> char *name,
>  
>  	rinfo = &req->r_reply_info;
>  	if (!IS_ENCRYPTED(dir)) {
> -		memcpy(name, rinfo->dname, rinfo->dname_len);
> -		name[rinfo->dname_len] = 0;
> +		err = ceph_export_copy_name(name, rinfo->dname,
> +					    rinfo->dname_len);
>  	} else {
>  		struct fscrypt_str oname = FSTR_INIT(NULL, 0);
>  		struct ceph_fname fname = { .dir	= dir,
> @@ -595,10 +604,9 @@ static int ceph_get_name(struct dentry *parent,
> char *name,
>  			goto out;
>  
>  		err = ceph_fname_to_usr(&fname, NULL, &oname, NULL);
> -		if (!err) {
> -			memcpy(name, oname.name, oname.len);
> -			name[oname.len] = 0;
> -		}
> +		if (!err)
> +			err = ceph_export_copy_name(name,
> oname.name,
> +						    oname.len);
>  		ceph_fname_free_buffer(dir, &oname);
>  	}
>  out:

Looks good from my point of view.

Reviewed-by: Viacheslav Dubeyko <slava@xxxxxxxxxxx>

Thanks,
Slava.