Re: [PATCH v3 8/9] fuse: add extent map I/O support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] fuse: fsync skips CPU cache flushing and metadata sync for EXTMAP DAX
- [High] fuse: dax_iomap_fault stubs return -EIO instead of valid vm_fault_t
- [Medium] fuse: incorrect -EFBIG error on write past EOF for EXTMAP DAX
--

commit 6c74cf2f1c1750d6fe1df2fa7ef25325ee0d117e
Author: Miklos Szeredi <mszeredi@redhat.com>

fuse: add extent map I/O support

This commit wires up read, write, splice, and mmap operations for
extent-mapped files using the iomap and DAX infrastructure. When a
passthrough file is opened with an EXTMAP backing, I/O is dispatched
to the associated DAX devices.

> diff --git a/fs/fuse/ext_map.c b/fs/fuse/ext_map.c
> index 39d7bb521873c..4f418185c0b79 100644
> --- a/fs/fuse/ext_map.c
> +++ b/fs/fuse/ext_map.c
> @@ -3,6 +3,14 @@
>  #include "fuse_i.h"
>  #include <linux/rbtree.h>
>  #include <linux/pagemap.h>
> +#include <linux/iomap.h>
> +#include <linux/dax.h>
> +
> +#ifndef CONFIG_FS_DAX
> +#define dax_iomap_rw(...) (-EIO)
> +#define dax_iomap_fault(...) (-EIO)
> +#define dax_finish_sync_fault(...) (-EIO)
> +#endif

[Severity: High]
Can returning -EIO here cause memory management fault handler corruption? 

When CONFIG_FS_DAX is disabled, these macros stub out the DAX fault 
handlers to return -EIO. However, the return type of the fault handlers 
is vm_fault_t, which expects a bitmask of VM_FAULT_* flags (such as 
VM_FAULT_SIGBUS).

Returning a negative integer like -EIO is interpreted as setting almost
all VM_FAULT_x bits simultaneously. This might lead to unpredictable
kernel behavior or infinite fault loops when handled by the core memory
management fault logic.

[ ... ]

> +static vm_fault_t fuse_ext_map_huge_fault(struct vm_fault *vmf, unsigned int order)
> +{
> +	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	bool write_fault = (vmf->flags & FAULT_FLAG_WRITE) && (vmf->vma->vm_flags & VM_SHARED);
> +	vm_fault_t ret;
> +	unsigned long pfn;
> +
> +	if (WARN_ON_ONCE(!IS_DAX(inode)))
> +		return VM_FAULT_SIGBUS;
> +
[ ... ]
> +
> +	filemap_invalidate_lock_shared(inode->i_mapping);
> +
> +	ret = dax_iomap_fault(vmf, order, &pfn, NULL, &fuse_ext_map_iomap_ops);
> +	if (ret & VM_FAULT_NEEDDSYNC)
> +		ret = dax_finish_sync_fault(vmf, order, pfn);
> +
> +	filemap_invalidate_unlock_shared(inode->i_mapping);
> +
> +	if (write_fault)
> +		sb_end_pagefault(inode->i_sb);
> +
> +	return ret;
> +}

[ ... ]

> +ssize_t fuse_ext_map_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	ssize_t res;
> +
> +	res = generic_write_checks(iocb, from);
> +	if (res <= 0)
> +		return res;
> +
> +	fuse_rw_clamp(iocb, from);
> +
> +	if (!iov_iter_count(from))
> +		return -EFBIG;

[Severity: Medium]
Is -EFBIG the correct error code to return for writes past the end of an 
extent-mapped file? 

The fuse_rw_clamp() call truncates the iterator if the write starts at or
beyond the current file size. Returning -EFBIG in this condition maps a
standard EOF or out-of-space scenario to a file too large error. 

POSIX semantics generally expect -ENOSPC or 0 (a short write) when writing
past a non-extensible block device or fixed-size mapping. Returning -EFBIG
might cause applications like cp or dd to fail incorrectly or inadvertently
trigger a SIGXFSZ signal.

> +
> +	res = kiocb_modified(iocb);
> +	if (res)
> +		return res;
> +
> +	return dax_iomap_rw(iocb, from, &fuse_ext_map_iomap_ops);
> +}

[ ... ]

> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index b5bef7b9e466e..b5c66f63acd02 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -551,6 +551,10 @@ static int fuse_fsync(struct file *file, loff_t start, loff_t end,
>  	if (fuse_is_bad(inode))
>  		return -EIO;
>  
> +	/* Skip sync for DAX inodes */
> +	if (IS_DAX(inode) && !fuse_inode_vdax(inode))
> +		return 0;

[Severity: High]
Does this early return bypass CPU cache flushing and metadata sync for 
EXTMAP DAX files? 

For FUSE passthrough EXTMAP files, IS_DAX(inode) will be true, but 
fuse_inode_vdax(inode) evaluates to false since vdax is specific to the 
traditional virtio-fs DAX window. 

Returning 0 here completely skips file_write_and_wait_range(), which is
needed to flush CPU caches for DAX memory via dax_writeback_mapping_range().
It also skips sync_inode_metadata(), which syncs metadata updates to the 
FUSE server. This bypass could cause CPU-cached DAX writes to be lost on a
crash, violating DAX durability guarantees.

> +
>  	inode_lock(inode);
>  
>  	/*

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