diff mbox series

ext4: Fix WANRON caused by unconsistent boot loader inode's i_size and i_disksize

Message ID 20230308032643.641113-1-chengzhihao1@huawei.com
State New
Headers
Series ext4: Fix WANRON caused by unconsistent boot loader inode's i_size and i_disksize |

Commit Message

Zhihao Cheng March 8, 2023, 3:26 a.m. UTC 
  Using corrupted ext4 image(non-zero i_size for boot loader inode) could
trigger WARNON 'i_size_read(inode) < EXT4_I(inode)->i_disksize' in
ext4_handle_inode_extension():

 WARNING: CPU: 0 PID: 2580 at fs/ext4/file.c:319
 CPU: 0 PID: 2580 Comm: bb Not tainted 6.3.0-rc1-00004-g703695902cfa
 RIP: 0010:ext4_file_write_iter+0xbc7/0xd10
 Call Trace:
  vfs_write+0x3b1/0x5c0
  ksys_write+0x77/0x160
  __x64_sys_write+0x22/0x30
  do_syscall_64+0x39/0x80

Reproducer (See Link):
 1. mount corrupted ext4 image with non-zero i_size for boot loader inode
 2. ioctl(fd, EXT4_IOC_SWAP_BOOT)
 3. write(fd)  // O_DIRECT

Fix it by setting i_disksize while first loading boot loader inode.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217159
Cc: <stable@kernel.org>
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ext4/ioctl.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Theodore Ts'o March 8, 2023, 4:31 a.m. UTC | #1 
On Wed, Mar 08, 2023 at 11:26:43AM +0800, Zhihao Cheng wrote:
> Using corrupted ext4 image(non-zero i_size for boot loader inode) could
> trigger WARNON 'i_size_read(inode) < EXT4_I(inode)->i_disksize' in
> ext4_handle_inode_extension():
> 
>  WARNING: CPU: 0 PID: 2580 at fs/ext4/file.c:319
>  CPU: 0 PID: 2580 Comm: bb Not tainted 6.3.0-rc1-00004-g703695902cfa
>  RIP: 0010:ext4_file_write_iter+0xbc7/0xd10
>  Call Trace:
>   vfs_write+0x3b1/0x5c0
>   ksys_write+0x77/0x160
>   __x64_sys_write+0x22/0x30
>   do_syscall_64+0x39/0x80
> 
> Reproducer (See Link):
>  1. mount corrupted ext4 image with non-zero i_size for boot loader inode
>  2. ioctl(fd, EXT4_IOC_SWAP_BOOT)
>  3. write(fd)  // O_DIRECT
> 
> Fix it by setting i_disksize while first loading boot loader inode.

Thanks for reporting the bug, but this is not the correct fix.

We need to swap i_disksize when we swap i_size in swap_inode_data().
Otherwise, if we fail later in the swap_inode_boot_loader() function,
the change to i_datasize won't get undone, which will lead to further
problems.

The correct fix is here:

	https://lore.kernel.org/all/20230308041252.GC860405@mit.edu/

						- Ted

P.S.  Chrome refused to download the b.c attachment, claiming it was
"dangerous".  Perhaps it was because of the commands involving
system(3) which among other things, uses dd to overwrite /dev/sda with
the image file.

It's best if the reproducer program doesn't doesn't make assumption
about whether it's safe to randomly dd files to /dev/sda.  Of course,
I'm a paranoid s.o.b. so I'm not about to download, compile and
blindly run a random program that I get from the 'net.  :-)

But it's actually not all that convenient.  So I just deleted all of
the system(3) calls from your b.c program, and then used a simple
shell script:

     cp /vtmp/disk /vtmp/foo.img
     mount -o loop /vtmp/foo.img /mnt
     cd /mnt
     /vtmp/b

... where /vtmp in the guest VM is automatically setup if you are
using kvm-xfstests[1] to be a 9p file system passthrough of
/tmp/kvm-xfstests-$USER on the host.

[1] https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-xfstests.md
Theodore Ts'o March 11, 2023, 5:16 a.m. UTC | #2 
Actually, after looking more closely at swap_boot_loader_inode(), your
patch is better one.  I've dropped mine and applied yours, with commit
message clarified a bit:

    ext4: zero i_disksize when initializing the bootloader inode
    
    If the boot loader inode has never been used before, the
    EXT4_IOC_SWAP_BOOT inode will initialize it, including setting the
    i_size to 0.  However, if the "never before used" boot loader has a
    non-zero i_size, then i_disksize will be non-zero, and the
    inconsistency between i_size and i_disksize can trigger a kernel
    warning:
    
     WARNING: CPU: 0 PID: 2580 at fs/ext4/file.c:319
     CPU: 0 PID: 2580 Comm: bb Not tainted 6.3.0-rc1-00004-g703695902cfa
     RIP: 0010:ext4_file_write_iter+0xbc7/0xd10
     Call Trace:
      vfs_write+0x3b1/0x5c0
      ksys_write+0x77/0x160
      __x64_sys_write+0x22/0x30
      do_syscall_64+0x39/0x80
    
    Reproducer:
     1. create corrupted image and mount it:
           mke2fs -t ext4 /tmp/foo.img 200
           debugfs -wR "sif <5> size 25700" /tmp/foo.img
           mount -t ext4 /tmp/foo.img /mnt
           cd /mnt
           echo 123 > file
     2. Run the reproducer program:
           posix_memalign(&buf, 1024, 1024)
           fd = open("file", O_RDWR | O_DIRECT);
           ioctl(fd, EXT4_IOC_SWAP_BOOT);
           write(fd, buf, 1024);
    
    Fix this by setting i_disksize as well as i_size to zero when
    initiaizing the boot loader inode.
    
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=217159
    Cc: stable@kernel.org
    Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
    Link: https://lore.kernel.org/r/20230308032643.641113-1-chengzhihao1@huawei.com
    Signed-off-by: Theodore Ts'o <tytso@mit.edu>
diff mbox series

Patch

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 12435d61f09e..f9a430152063 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -431,6 +431,7 @@  static long swap_inode_boot_loader(struct super_block *sb,
 		ei_bl->i_flags = 0;
 		inode_set_iversion(inode_bl, 1);
 		i_size_write(inode_bl, 0);
+		EXT4_I(inode_bl)->i_disksize = inode_bl->i_size;
 		inode_bl->i_mode = S_IFREG;
 		if (ext4_has_feature_extents(sb)) {
 			ext4_set_inode_flag(inode_bl, EXT4_INODE_EXTENTS);