Re: [PATCH] ext4: Fix WANRON caused by unconsistent boot loader inode's i_size and i_disksize

[Theodore Ts'o]

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@xxxxxxx/

						- 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