| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260122223119.GB5900@frogsfrogsfrogs>
Date: Thu, 22 Jan 2026 14:31:19 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Joanne Koong <joannelkoong@...il.com>
Cc: miklos@...redi.hu, bernd@...ernd.com, neal@...pa.dev,
linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 10/31] fuse: implement basic iomap reporting such as
FIEMAP and SEEK_{DATA,HOLE}
On Wed, Jan 21, 2026 at 06:07:12PM -0800, Joanne Koong wrote:
> On Tue, Oct 28, 2025 at 5:47 PM Darrick J. Wong <djwong@...nel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@...nel.org>
> >
> > Implement the basic file mapping reporting functions like FIEMAP, BMAP,
> > and SEEK_DATA/HOLE.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>
> > ---
> > fs/fuse/fuse_i.h | 8 ++++++
> > fs/fuse/dir.c | 1 +
> > fs/fuse/file.c | 13 ++++++++++
> > fs/fuse/file_iomap.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 89 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index c7aeb324fe599e..6fe8aa1845b98d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1730,6 +1730,11 @@ static inline bool fuse_inode_has_iomap(const struct inode *inode)
> >
> > return test_bit(FUSE_I_IOMAP, &fi->state);
> > }
> > +
> > +int fuse_iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > + u64 start, u64 length);
> > +loff_t fuse_iomap_lseek(struct file *file, loff_t offset, int whence);
> > +sector_t fuse_iomap_bmap(struct address_space *mapping, sector_t block);
> > #else
> > # define fuse_iomap_enabled(...) (false)
> > # define fuse_has_iomap(...) (false)
> > @@ -1739,6 +1744,9 @@ static inline bool fuse_inode_has_iomap(const struct inode *inode)
> > # define fuse_iomap_init_nonreg_inode(...) ((void)0)
> > # define fuse_iomap_evict_inode(...) ((void)0)
> > # define fuse_inode_has_iomap(...) (false)
> > +# define fuse_iomap_fiemap NULL
> > +# define fuse_iomap_lseek(...) (-ENOSYS)
> > +# define fuse_iomap_bmap(...) (-ENOSYS)
> > #endif
> >
> > #endif /* _FS_FUSE_I_H */
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index 18eb1bb192bb58..bafc386f2f4d3a 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -2296,6 +2296,7 @@ static const struct inode_operations fuse_common_inode_operations = {
> > .set_acl = fuse_set_acl,
> > .fileattr_get = fuse_fileattr_get,
> > .fileattr_set = fuse_fileattr_set,
> > + .fiemap = fuse_iomap_fiemap,
> > };
> >
> > static const struct inode_operations fuse_symlink_inode_operations = {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index bd9c208a46c78d..8a981f41b1dbd0 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -2512,6 +2512,12 @@ static sector_t fuse_bmap(struct address_space *mapping, sector_t block)
> > struct fuse_bmap_out outarg;
> > int err;
> >
> > + if (fuse_inode_has_iomap(inode)) {
> > + sector_t alt_sec = fuse_iomap_bmap(mapping, block);
> > + if (alt_sec > 0)
> > + return alt_sec;
> > + }
> > +
> > if (!inode->i_sb->s_bdev || fm->fc->no_bmap)
> > return 0;
> >
> > @@ -2547,6 +2553,13 @@ static loff_t fuse_lseek(struct file *file, loff_t offset, int whence)
> > struct fuse_lseek_out outarg;
> > int err;
> >
> > + if (fuse_inode_has_iomap(inode)) {
> > + loff_t alt_pos = fuse_iomap_lseek(file, offset, whence);
> > +
> > + if (alt_pos >= 0 || (alt_pos < 0 && alt_pos != -ENOSYS))
>
> I don't think you technically need the "alt_pos < 0" part here since
> the "alt_pos >= 0 ||" part already accounts for that
alt_pos is loff_t, which is a signed type.
But I think this could be more concise:
alt_pos = fuse_iomap_lseek(...);
if (alt_pos != -ENOSYS)
return alt_pos;
> > + return alt_pos;
> > + }
> > +
> > if (fm->fc->no_lseek)
> > goto fallback;
> >
> > diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> > index 66a7b8faa31ac2..ce64e7c4860ef8 100644
> > --- a/fs/fuse/file_iomap.c
> > +++ b/fs/fuse/file_iomap.c
> > @@ -4,6 +4,7 @@
> > * Author: Darrick J. Wong <djwong@...nel.org>
> > */
> > #include <linux/iomap.h>
> > +#include <linux/fiemap.h>
> > #include "fuse_i.h"
> > #include "fuse_trace.h"
> > #include "iomap_i.h"
> > @@ -561,7 +562,7 @@ static int fuse_iomap_end(struct inode *inode, loff_t pos, loff_t count,
> > return err;
> > }
> >
> > -const struct iomap_ops fuse_iomap_ops = {
> > +static const struct iomap_ops fuse_iomap_ops = {
> > .iomap_begin = fuse_iomap_begin,
> > .iomap_end = fuse_iomap_end,
> > };
> > @@ -690,3 +691,68 @@ void fuse_iomap_evict_inode(struct inode *inode)
> > if (conn->iomap && fuse_inode_is_exclusive(inode))
> > clear_bit(FUSE_I_EXCLUSIVE, &fi->state);
> > }
> > +
> > +int fuse_iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > + u64 start, u64 count)
> > +{
> > + struct fuse_conn *fc = get_fuse_conn(inode);
> > + int error;
> > +
> > + /*
> > + * We are called directly from the vfs so we need to check per-inode
> > + * support here explicitly.
> > + */
> > + if (!fuse_inode_has_iomap(inode))
> > + return -EOPNOTSUPP;
> > +
> > + if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR)
>
> I don't see where FIEMAP_FLAG_SYNC and FIEMAP_FLAG_CACHE are supported
> either, should these return -EOPNOTSUPP if they're set as well?
The vfs implements FIEMAP_FLAG_SYNC for us in fiemap_prep, which is
called by iomap_fiemap.
I'm not sure what FIEMAP_FLAG_CACHE means in this context. Its comment
says "request caching of the extents" which doesn't sound like doing
anything is mandatory.
> > + return -EOPNOTSUPP;
> > +
> > + if (fuse_is_bad(inode))
> > + return -EIO;
> > +
> > + if (!fuse_allow_current_process(fc))
> > + return -EACCES;
> > +
> > + inode_lock_shared(inode);
> > + error = iomap_fiemap(inode, fieinfo, start, count, &fuse_iomap_ops);
> > + inode_unlock_shared(inode);
> > +
> > + return error;
> > +}
> > +
> > +sector_t fuse_iomap_bmap(struct address_space *mapping, sector_t block)
> > +{
> > + ASSERT(fuse_inode_has_iomap(mapping->host));
> > +
> > + return iomap_bmap(mapping, block, &fuse_iomap_ops);
> > +}
> > +
> > +loff_t fuse_iomap_lseek(struct file *file, loff_t offset, int whence)
> > +{
> > + struct inode *inode = file->f_mapping->host;
> > + struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > + ASSERT(fuse_inode_has_iomap(inode));
> > +
> > + if (fuse_is_bad(inode))
> > + return -EIO;
> > +
> > + if (!fuse_allow_current_process(fc))
> > + return -EACCES;
> > +
> > + switch (whence) {
> > + case SEEK_HOLE:
> > + offset = iomap_seek_hole(inode, offset, &fuse_iomap_ops);
> > + break;
> > + case SEEK_DATA:
> > + offset = iomap_seek_data(inode, offset, &fuse_iomap_ops);
> > + break;
> > + default:
>
> Does it make sense to have the default case just call generic_file_llseek()?
Yes. Thanks for spotting that bug!
--D
> Thanks,
> Joanne
>
> > + return -ENOSYS;
> > + }
> > +
> > + if (offset < 0)
> > + return offset;
> > + return vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
> > +}
> >
>
Powered by blists - more mailing lists