| Message ID | 20250417064042.712140-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs: move the bdex_statx call to vfs_getattr_nosec | expand |
On Thu, Apr 17, 2025 at 08:40:42AM +0200, Christoph Hellwig wrote: > Currently bdex_statx is only called from the very high-level > vfs_statx_path function, and thus bypassing it for in-kernel calls > to vfs_getattr or vfs_getattr_nosec. > > This breaks querying the block ѕize of the underlying device in the > loop driver and also is a pitfall for any other new kernel caller. > > Move the call into the lowest level helper to ensure all callers get > the right results. > > Fixes: 2d985f8c6b91 ("vfs: support STATX_DIOALIGN on block devices") > Fixes: f4774e92aab8 ("loop: take the file system minimum dio alignment into account") > Reported-by: Darrick J. Wong <djwong@kernel.org> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- This looks fine but one thing below. > block/bdev.c | 3 +-- > fs/stat.c | 32 ++++++++++++++++++-------------- > include/linux/blkdev.h | 6 +++--- > 3 files changed, 22 insertions(+), 19 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index 4844d1e27b6f..6a34179192c9 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -1272,8 +1272,7 @@ void sync_bdevs(bool wait) > /* > * Handle STATX_{DIOALIGN, WRITE_ATOMIC} for block devices. > */ > -void bdev_statx(struct path *path, struct kstat *stat, > - u32 request_mask) > +void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask) > { > struct inode *backing_inode; > struct block_device *bdev; > diff --git a/fs/stat.c b/fs/stat.c > index f13308bfdc98..3d9222807214 100644 > --- a/fs/stat.c > +++ b/fs/stat.c > @@ -204,12 +204,25 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat, > STATX_ATTR_DAX); > > idmap = mnt_idmap(path->mnt); > - if (inode->i_op->getattr) > - return inode->i_op->getattr(idmap, path, stat, > - request_mask, > - query_flags); > + if (inode->i_op->getattr) { > + int ret; > + > + ret = inode->i_op->getattr(idmap, path, stat, request_mask, > + query_flags); > + if (ret) > + return ret; > + } else { > + generic_fillattr(idmap, request_mask, inode, stat); > + } > + > + /* > + * If this is a block device inode, override the filesystem attributes > + * with the block device specific parameters that need to be obtained > + * from the bdev backing inode. > + */ > + if (S_ISBLK(stat->mode)) > + bdev_statx(path, stat, request_mask); > > - generic_fillattr(idmap, request_mask, inode, stat); > return 0; > } > EXPORT_SYMBOL(vfs_getattr_nosec); > @@ -295,15 +308,6 @@ static int vfs_statx_path(struct path *path, int flags, struct kstat *stat, > if (path_mounted(path)) > stat->attributes |= STATX_ATTR_MOUNT_ROOT; > stat->attributes_mask |= STATX_ATTR_MOUNT_ROOT; > - > - /* > - * If this is a block device inode, override the filesystem > - * attributes with the block device specific parameters that need to be > - * obtained from the bdev backing inode. > - */ > - if (S_ISBLK(stat->mode)) > - bdev_statx(path, stat, request_mask); bdev_statx() -> blkdev_get_no_open() { if (!inode && IS_ENABLED(CONFIG_BLOCK_LEGACY_AUTOLOAD)) { blk_request_module(dev); inode = ilookup(blockdev_superblock, dev); if (inode) pr_warn_ratelimited("block device autoloading is deprecated and will be removed.\n"); } } So that means any unprivileged Schmock can trigger a module autoload from statx() if that's enabled? It feels like this should really be disabled for statx().
On Thu, 17 Apr 2025 08:40:42 +0200, Christoph Hellwig wrote: > Currently bdex_statx is only called from the very high-level > vfs_statx_path function, and thus bypassing it for in-kernel calls > to vfs_getattr or vfs_getattr_nosec. > > This breaks querying the block ѕize of the underlying device in the > loop driver and also is a pitfall for any other new kernel caller. > > [...] Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.fixes [1/1] fs: move the bdex_statx call to vfs_getattr_nosec https://git.kernel.org/vfs/vfs/c/777d0961ff95
On Thu, Apr 17, 2025 at 08:40:42AM +0200, Christoph Hellwig wrote: > Currently bdex_statx is only called from the very high-level > vfs_statx_path function, and thus bypassing it for in-kernel calls > to vfs_getattr or vfs_getattr_nosec. > > This breaks querying the block ѕize of the underlying device in the > loop driver and also is a pitfall for any other new kernel caller. > > Move the call into the lowest level helper to ensure all callers get > the right results. > > Fixes: 2d985f8c6b91 ("vfs: support STATX_DIOALIGN on block devices") > Fixes: f4774e92aab8 ("loop: take the file system minimum dio alignment into account") > Reported-by: Darrick J. Wong <djwong@kernel.org> > Signed-off-by: Christoph Hellwig <hch@lst.de> This appears to solve the problem as well. Tested-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > block/bdev.c | 3 +-- > fs/stat.c | 32 ++++++++++++++++++-------------- > include/linux/blkdev.h | 6 +++--- > 3 files changed, 22 insertions(+), 19 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index 4844d1e27b6f..6a34179192c9 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -1272,8 +1272,7 @@ void sync_bdevs(bool wait) > /* > * Handle STATX_{DIOALIGN, WRITE_ATOMIC} for block devices. > */ > -void bdev_statx(struct path *path, struct kstat *stat, > - u32 request_mask) > +void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask) > { > struct inode *backing_inode; > struct block_device *bdev; > diff --git a/fs/stat.c b/fs/stat.c > index f13308bfdc98..3d9222807214 100644 > --- a/fs/stat.c > +++ b/fs/stat.c > @@ -204,12 +204,25 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat, > STATX_ATTR_DAX); > > idmap = mnt_idmap(path->mnt); > - if (inode->i_op->getattr) > - return inode->i_op->getattr(idmap, path, stat, > - request_mask, > - query_flags); > + if (inode->i_op->getattr) { > + int ret; > + > + ret = inode->i_op->getattr(idmap, path, stat, request_mask, > + query_flags); > + if (ret) > + return ret; > + } else { > + generic_fillattr(idmap, request_mask, inode, stat); > + } > + > + /* > + * If this is a block device inode, override the filesystem attributes > + * with the block device specific parameters that need to be obtained > + * from the bdev backing inode. > + */ > + if (S_ISBLK(stat->mode)) > + bdev_statx(path, stat, request_mask); > > - generic_fillattr(idmap, request_mask, inode, stat); > return 0; > } > EXPORT_SYMBOL(vfs_getattr_nosec); > @@ -295,15 +308,6 @@ static int vfs_statx_path(struct path *path, int flags, struct kstat *stat, > if (path_mounted(path)) > stat->attributes |= STATX_ATTR_MOUNT_ROOT; > stat->attributes_mask |= STATX_ATTR_MOUNT_ROOT; > - > - /* > - * If this is a block device inode, override the filesystem > - * attributes with the block device specific parameters that need to be > - * obtained from the bdev backing inode. > - */ > - if (S_ISBLK(stat->mode)) > - bdev_statx(path, stat, request_mask); > - > return 0; > } > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index e39c45bc0a97..678dc38442bf 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1685,7 +1685,7 @@ int sync_blockdev(struct block_device *bdev); > int sync_blockdev_range(struct block_device *bdev, loff_t lstart, loff_t lend); > int sync_blockdev_nowait(struct block_device *bdev); > void sync_bdevs(bool wait); > -void bdev_statx(struct path *, struct kstat *, u32); > +void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask); > void printk_all_partitions(void); > int __init early_lookup_bdev(const char *pathname, dev_t *dev); > #else > @@ -1703,8 +1703,8 @@ static inline int sync_blockdev_nowait(struct block_device *bdev) > static inline void sync_bdevs(bool wait) > { > } > -static inline void bdev_statx(struct path *path, struct kstat *stat, > - u32 request_mask) > +static inline void bdev_statx(const struct path *path, struct kstat *stat, > + u32 request_mask) > { > } > static inline void printk_all_partitions(void) > -- > 2.47.2 > >
diff --git a/block/bdev.c b/block/bdev.c index 4844d1e27b6f..6a34179192c9 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -1272,8 +1272,7 @@ void sync_bdevs(bool wait) /* * Handle STATX_{DIOALIGN, WRITE_ATOMIC} for block devices. */ -void bdev_statx(struct path *path, struct kstat *stat, - u32 request_mask) +void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask) { struct inode *backing_inode; struct block_device *bdev; diff --git a/fs/stat.c b/fs/stat.c index f13308bfdc98..3d9222807214 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -204,12 +204,25 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat, STATX_ATTR_DAX); idmap = mnt_idmap(path->mnt); - if (inode->i_op->getattr) - return inode->i_op->getattr(idmap, path, stat, - request_mask, - query_flags); + if (inode->i_op->getattr) { + int ret; + + ret = inode->i_op->getattr(idmap, path, stat, request_mask, + query_flags); + if (ret) + return ret; + } else { + generic_fillattr(idmap, request_mask, inode, stat); + } + + /* + * If this is a block device inode, override the filesystem attributes + * with the block device specific parameters that need to be obtained + * from the bdev backing inode. + */ + if (S_ISBLK(stat->mode)) + bdev_statx(path, stat, request_mask); - generic_fillattr(idmap, request_mask, inode, stat); return 0; } EXPORT_SYMBOL(vfs_getattr_nosec); @@ -295,15 +308,6 @@ static int vfs_statx_path(struct path *path, int flags, struct kstat *stat, if (path_mounted(path)) stat->attributes |= STATX_ATTR_MOUNT_ROOT; stat->attributes_mask |= STATX_ATTR_MOUNT_ROOT; - - /* - * If this is a block device inode, override the filesystem - * attributes with the block device specific parameters that need to be - * obtained from the bdev backing inode. - */ - if (S_ISBLK(stat->mode)) - bdev_statx(path, stat, request_mask); - return 0; } diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index e39c45bc0a97..678dc38442bf 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1685,7 +1685,7 @@ int sync_blockdev(struct block_device *bdev); int sync_blockdev_range(struct block_device *bdev, loff_t lstart, loff_t lend); int sync_blockdev_nowait(struct block_device *bdev); void sync_bdevs(bool wait); -void bdev_statx(struct path *, struct kstat *, u32); +void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask); void printk_all_partitions(void); int __init early_lookup_bdev(const char *pathname, dev_t *dev); #else @@ -1703,8 +1703,8 @@ static inline int sync_blockdev_nowait(struct block_device *bdev) static inline void sync_bdevs(bool wait) { } -static inline void bdev_statx(struct path *path, struct kstat *stat, - u32 request_mask) +static inline void bdev_statx(const struct path *path, struct kstat *stat, + u32 request_mask) { } static inline void printk_all_partitions(void)
Currently bdex_statx is only called from the very high-level vfs_statx_path function, and thus bypassing it for in-kernel calls to vfs_getattr or vfs_getattr_nosec. This breaks querying the block ѕize of the underlying device in the loop driver and also is a pitfall for any other new kernel caller. Move the call into the lowest level helper to ensure all callers get the right results. Fixes: 2d985f8c6b91 ("vfs: support STATX_DIOALIGN on block devices") Fixes: f4774e92aab8 ("loop: take the file system minimum dio alignment into account") Reported-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bdev.c | 3 +-- fs/stat.c | 32 ++++++++++++++++++-------------- include/linux/blkdev.h | 6 +++--- 3 files changed, 22 insertions(+), 19 deletions(-)