Re: [PATCH vfs.all 00/26] fs & block: remove bdev->bd_inode

[Yu Kuai <yukuai1@huaweicloud.com>]

Hi, Christian!
Hi, Jan!
+CC Gustavo

While testing this set, I found that the branch vfs.all seems broken,
xfstests report success while lots of BUG is reported in dmesg:

[22709.079704] 
=============================================================================^M
[22709.082404] BUG kmalloc-16 (Not tainted): Right Redzone overwritten^M
[22709.084148] 
-----------------------------------------------------------------------------^M
[22709.084148] ^M
[22709.086784] 0xffff88817d52e7a0-0xffff88817d52e7a7 @offset=1952. First 
byte 0x0 instead of 0xcc^M
[22709.089169] Allocated in do_handle_open+0x97/0x440 age=10 cpu=13 
pid=814795^M
[22709.091158]  __kmalloc+0x41d/0x5e0^M
[22709.092153]  do_handle_open+0x97/0x440^M
[22709.093240]  __x64_sys_open_by_handle_at+0x23/0x30^M
[22709.094482]  do_syscall_64+0xb1/0x210^M
[22709.095316]  entry_SYSCALL_64_after_hwframe+0x6c/0x74^M
[22709.096414] Freed in kvfree+0x4c/0x60 age=43560 cpu=15 pid=813506^M
[22709.097719]  kfree+0x31c/0x530^M
[22709.098396]  kvfree+0x4c/0x60^M
[22709.099048]  ext4_mb_release+0x29c/0x570^M
[22709.099901]  ext4_put_super+0x17f/0x590^M
[22709.100735]  generic_shutdown_super+0xba/0x240^M
[22709.101698]  kill_block_super+0x22/0x70^M
[22709.102525]  ext4_kill_sb+0x2a/0x70^M
[22709.103297]  deactivate_locked_super+0x4f/0xe0^M
[22709.104261]  deactivate_super+0x81/0x90^M
[22709.104876]  cleanup_mnt+0xe0/0x1b0^M
[22709.105419]  __cleanup_mnt+0x1a/0x30^M
[22709.105964]  task_work_run+0x88/0x100^M
[22709.106531]  syscall_exit_to_user_mode+0x3cc/0x3e0^M
[22709.107263]  do_syscall_64+0xc5/0x210^M
[22709.107820]  entry_SYSCALL_64_after_hwframe+0x6c/0x74^M

While digging this problem, I found that commit 1b43c4629756 ("fs:
Annotate struct file_handle with __counted_by() and use struct_size()")
might made a mistake, and I verified following patch can fix the
problem.

Thanks,
Kuai

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 53ed54711cd2..bcfecac2dc54 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -201,8 +201,7 @@ static int handle_to_path(int mountdirfd, struct 
file_handle __user *ufh,
         /* copy the full handle */
         *handle = f_handle;
         if (copy_from_user(&handle->f_handle,
-                          &ufh->f_handle,
-                          struct_size(ufh, f_handle, 
f_handle.handle_bytes))) {
+                          &ufh->f_handle, f_handle.handle_bytes)) {
                 retval = -EFAULT;
                 goto out_handle;
         }

在 2024/04/06 17:09, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Hi, Jens!
> Hi, Jan!
> Hi, Christoph!
> Hi, Christian!
> Hi, AL!
> 
> Sorry for the delay(I was overwhelmed with other work stuff). Main changes
> from last version is patch 22(modified based on [1]), the idea is that
> stash a 'bdev_file' in 'bd_inode->i_private' while opening bdev the first
> time, and release it when last opener close the bdev.
> 
> The patch to use bdev and bdev_file as union for iomap/buffer_head is
> dropped and changes for iomap/buffer is splitted to patch 23-26.
> 
> I tested this set in my VM with blktests for virtio-scsi and xfstests
> for ext4/xfs for one round now, no regerssions are found yet.
> 
> Please let me know what you think!
> 
> [1] https://lore.kernel.org/all/c62dac0e-666f-9cc9-cffe-f3d985029d6a@huaweicloud.com/
> 
> Changes from RFC v4:
>   - respin on the top of vfs.all branch from vfs tree;
>   - add review tag, patches that are not reviewed: patch 19-26;
>   - add patch 21, fix a module reference problem;
>   - instead of using a union of bdev(for raw block device) and
>   bdev_file(for filesystems), add patch 22 to stash a bdev_file to
>   bd_inode->i_private, so that iomap and buffer_head for raw block device
>   can convert to use bdev_file as well;
>   - split the huge path for iomap/buffer into 4 patches, 21-24;
> 
> Changes from RFC v3:
>   - respin on the top of linux-next, based on Christian's patchset to
>   open bdev as file. Most of patches from v3 is dropped and change to use
>   file_inode(bdev_file) to get bd_inode or bdev_file->f_mapping to get
>   bd_inode->i_mapping.
> 
> Changes from RFC v2:
>   - remove bdev_associated_mapping() and patch 12 from v1;
>   - add kerneldoc comments for new bdev apis;
>   - rename __bdev_get_folio() to bdev_get_folio;
>   - fix a problem in erofs that erofs_init_metabuf() is not always
>   called.
>   - add reviewed-by tag for patch 15-17;
> 
> Changes from RFC v1:
>   - remove some bdev apis that is not necessary;
>   - pass in offset for bdev_read_folio() and __bdev_get_folio();
>   - remove bdev_gfp_constraint() and add a new helper in fs/buffer.c to
>   prevent access bd_indoe() directly from mapping_gfp_constraint() in
>   ext4.(patch 15, 16);
>   - remove block_device_ejected() from ext4.
> 
> Yu Kuai (26):
>    block: move two helpers into bdev.c
>    block: remove sync_blockdev_nowait()
>    block: remove sync_blockdev_range()
>    block: prevent direct access of bd_inode
>    block: add a helper bdev_read_folio()
>    bcachefs: remove dead function bdev_sectors()
>    cramfs: prevent direct access of bd_inode
>    erofs: prevent direct access of bd_inode
>    nilfs2: prevent direct access of bd_inode
>    gfs2: prevent direct access of bd_inode
>    btrfs: prevent direct access of bd_inode
>    ext4: remove block_device_ejected()
>    ext4: prevent direct access of bd_inode
>    jbd2: prevent direct access of bd_inode
>    s390/dasd: use bdev api in dasd_format()
>    bcache: prevent direct access of bd_inode
>    block2mtd: prevent direct access of bd_inode
>    scsi: use bdev helper in scsi_bios_ptable()
>    dm-vdo: convert to use bdev_file
>    block: factor out a helper init_bdev_file()
>    block: fix module reference leakage from bdev_open_by_dev error path
>    block: stash a bdev_file to read/write raw blcok_device
>    iomap: add helpers helpers to get and set bdev
>    iomap: convert to use bdev_file
>    buffer: add helpers to get and set bdev
>    buffer: convert to use bdev_file
> 
>   block/bdev.c                              | 262 ++++++++++++++++------
>   block/blk-zoned.c                         |   4 +-
>   block/blk.h                               |   2 +
>   block/fops.c                              |   6 +-
>   block/genhd.c                             |   9 +-
>   block/ioctl.c                             |   8 +-
>   block/partitions/core.c                   |   8 +-
>   drivers/md/bcache/super.c                 |   7 +-
>   drivers/md/dm-vdo/dedupe.c                |   7 +-
>   drivers/md/dm-vdo/dm-vdo-target.c         |   9 +-
>   drivers/md/dm-vdo/indexer/config.c        |   2 +-
>   drivers/md/dm-vdo/indexer/config.h        |   4 +-
>   drivers/md/dm-vdo/indexer/index-layout.c  |   6 +-
>   drivers/md/dm-vdo/indexer/index-layout.h  |   2 +-
>   drivers/md/dm-vdo/indexer/index-session.c |  18 +-
>   drivers/md/dm-vdo/indexer/index.c         |   4 +-
>   drivers/md/dm-vdo/indexer/index.h         |   2 +-
>   drivers/md/dm-vdo/indexer/indexer.h       |   6 +-
>   drivers/md/dm-vdo/indexer/io-factory.c    |  17 +-
>   drivers/md/dm-vdo/indexer/io-factory.h    |   4 +-
>   drivers/md/dm-vdo/indexer/volume.c        |   4 +-
>   drivers/md/dm-vdo/indexer/volume.h        |   2 +-
>   drivers/md/dm-vdo/vdo.c                   |   2 +-
>   drivers/md/md-bitmap.c                    |   2 +-
>   drivers/mtd/devices/block2mtd.c           |   6 +-
>   drivers/s390/block/dasd_ioctl.c           |   5 +-
>   drivers/scsi/scsicam.c                    |   3 +-
>   fs/affs/file.c                            |   2 +-
>   fs/bcachefs/util.h                        |   5 -
>   fs/btrfs/disk-io.c                        |  17 +-
>   fs/btrfs/disk-io.h                        |   4 +-
>   fs/btrfs/inode.c                          |   2 +-
>   fs/btrfs/super.c                          |   2 +-
>   fs/btrfs/volumes.c                        |  25 ++-
>   fs/btrfs/zoned.c                          |  20 +-
>   fs/btrfs/zoned.h                          |   4 +-
>   fs/buffer.c                               | 104 ++++-----
>   fs/cramfs/inode.c                         |   2 +-
>   fs/direct-io.c                            |   4 +-
>   fs/erofs/data.c                           |  22 +-
>   fs/erofs/internal.h                       |   1 +
>   fs/erofs/zmap.c                           |   2 +-
>   fs/exfat/fatent.c                         |   2 +-
>   fs/ext2/inode.c                           |   4 +-
>   fs/ext2/xattr.c                           |   2 +-
>   fs/ext4/dir.c                             |   2 +-
>   fs/ext4/ext4_jbd2.c                       |   2 +-
>   fs/ext4/inode.c                           |   2 +-
>   fs/ext4/mmp.c                             |   2 +-
>   fs/ext4/page-io.c                         |   5 +-
>   fs/ext4/super.c                           |  30 +--
>   fs/ext4/xattr.c                           |   2 +-
>   fs/f2fs/data.c                            |  10 +-
>   fs/f2fs/f2fs.h                            |   1 +
>   fs/fat/inode.c                            |   2 +-
>   fs/fuse/dax.c                             |   2 +-
>   fs/gfs2/aops.c                            |   2 +-
>   fs/gfs2/bmap.c                            |   2 +-
>   fs/gfs2/glock.c                           |   2 +-
>   fs/gfs2/meta_io.c                         |   2 +-
>   fs/gfs2/ops_fstype.c                      |   2 +-
>   fs/hpfs/file.c                            |   2 +-
>   fs/iomap/buffered-io.c                    |   8 +-
>   fs/iomap/direct-io.c                      |  11 +-
>   fs/iomap/swapfile.c                       |   2 +-
>   fs/iomap/trace.h                          |   6 +-
>   fs/jbd2/commit.c                          |   2 +-
>   fs/jbd2/journal.c                         |  34 +--
>   fs/jbd2/recovery.c                        |   9 +-
>   fs/jbd2/revoke.c                          |  14 +-
>   fs/jbd2/transaction.c                     |   8 +-
>   fs/mpage.c                                |  18 +-
>   fs/nilfs2/btnode.c                        |   4 +-
>   fs/nilfs2/gcinode.c                       |   2 +-
>   fs/nilfs2/mdt.c                           |   2 +-
>   fs/nilfs2/page.c                          |   4 +-
>   fs/nilfs2/recovery.c                      |  27 ++-
>   fs/nilfs2/segment.c                       |   2 +-
>   fs/ntfs3/fsntfs.c                         |  10 +-
>   fs/ntfs3/inode.c                          |   4 +-
>   fs/ntfs3/super.c                          |   6 +-
>   fs/ocfs2/journal.c                        |   2 +-
>   fs/reiserfs/fix_node.c                    |   2 +-
>   fs/reiserfs/journal.c                     |  10 +-
>   fs/reiserfs/prints.c                      |   4 +-
>   fs/reiserfs/reiserfs.h                    |   6 +-
>   fs/reiserfs/stree.c                       |   2 +-
>   fs/reiserfs/tail_conversion.c             |   2 +-
>   fs/sync.c                                 |   9 +-
>   fs/xfs/xfs_iomap.c                        |   4 +-
>   fs/zonefs/file.c                          |   4 +-
>   include/linux/blk_types.h                 |   2 +-
>   include/linux/blkdev.h                    |  19 +-
>   include/linux/buffer_head.h               |  81 ++++---
>   include/linux/iomap.h                     |  13 +-
>   include/linux/jbd2.h                      |  18 +-
>   include/trace/events/block.h              |   2 +-
>   97 files changed, 620 insertions(+), 440 deletions(-)
>