Re: [PATCH v3 6/9] fuse: add support for opening dax device as backing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] fuse: FUSE_ATTR_DAX applied to non-regular files causes symlink DoS
- [High] fuse: sleep-in-atomic bug in fuse_backing_free() during RCU inode teardown
--

commit 122a624f241dd19a7e4f459c49f52f0a5d738881
Author: Miklos Szeredi <mszeredi@redhat.com>
Subject: fuse: add support for opening dax device as backing

This commit adds support for using a DAX device as a backing file in FUSE
passthrough mode. It introduces a new DAX backing type and marks the inode
with S_DAX if FUSE_ATTR_DAX is set during lookup.

> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> index bc40818778dff..0ed850ddf8cde 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
[ ... ]
> @@ -22,9 +23,16 @@ static void fuse_backing_free(struct fuse_backing *fb)
>  {
>  	pr_debug("%s: fb=0x%p\n", __func__, fb);
>  
> -	if (fb->file)
> -		fput(fb->file);
> -	put_cred(fb->cred);
> +	switch (fb->type) {
> +	case FUSE_BACKING_PATH:
> +		path_put(&fb->path);
> +		put_cred(fb->cred);
> +		break;
> +
> +	case FUSE_BACKING_DAXDEV:
> +		fs_put_dax(fb->dax_dev, fb);
> +		break;
> +	}

[Severity: High]
Can path_put() or fs_put_dax() be called from atomic context here?

When the final reference is dropped during inode teardown in
fuse_free_inode(), it executes from the VFS RCU softirq callback
(i_callback).

If fuse_backing_put() is called from that RCU softirq context and drops
the final reference, invoking fuse_backing_free() synchronously, could
the sleeping functions path_put() and fs_put_dax() (which calls iput())
trigger a sleep-in-atomic bug?

>  	kfree_rcu(fb, rcu);
>  }
>  
[ ... ]
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index b5b51865d59f8..750e092c3971d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
[ ... ]
> @@ -430,6 +434,11 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr,
>  	 */
>  	if (!fc->posix_acl)
>  		inode->i_acl = inode->i_default_acl = ACL_DONT_CACHE;
> +
> +	if ((attr->flags & FUSE_ATTR_DAX) && !fuse_inode_vdax(inode)) {
> +		inode->i_flags |= S_DAX;
> +		inode->i_data.a_ops = &fuse_dax_aops;
> +	}

[Severity: Critical]
Does this overwrite the address space operations for non-regular files
like symlinks?

If a FUSE server returns FUSE_ATTR_DAX on a symlink lookup, this code
unconditionally sets inode->i_data.a_ops = &fuse_dax_aops.

Since fuse_dax_aops does not implement a .read_folio callback, won't
this cause a NULL pointer dereference when a readlink() syscall attempts
to read the symlink (e.g., via fuse_get_link() -> page_get_link_raw() ->
read_mapping_folio())?

Should there be a check for S_ISREG(inode->i_mode) before setting the
DAX address space operations?

>  }
>  
>  static int fuse_inode_eq(struct inode *inode, void *_nodeidp)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006180115.1425232-1-mszeredi@redhat.com?part=6