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 <[email protected]> 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/[email protected]?part=8
