1. Patchew
  2. linux
  3. View series

[PATCH V4] loop: Add sanity check for read/write_iter

Lizhi Xu posted 1 patch 7 months, 3 weeks ago
There is a newer version of this series
drivers/block/loop.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
[PATCH V4] loop: Add sanity check for read/write_iter
Posted by Lizhi Xu 7 months, 3 weeks ago 
Some file systems do not support read_iter/write_iter, such as selinuxfs
in this issue.
So before calling them, first confirm that the interface is supported and
then call it.

It is releavant in that vfs_iter_read/write have the check, and removal
of their used caused szybot to be able to hit this issue.

Fixes: f2fed441c69b ("loop: stop using vfs_iter__{read,write} for buffered I/O")
Reported-by: syzbot+6af973a3b8dfd2faefdc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6af973a3b8dfd2faefdc
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V1 -> V2: move check to loop_configure and loop_change_fd
V2 -> V3: using helper for this check
V3 -> V4: remove input parameters change and mode

 drivers/block/loop.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 46cba261075f..655d33e63cb9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -505,6 +505,17 @@ static void loop_assign_backing_file(struct loop_device *lo, struct file *file)
 	lo->lo_min_dio_size = loop_query_min_dio_size(lo);
 }
 
+static int loop_check_backing_file(struct file *file)
+{
+	if (!file->f_op->read_iter)
+		return -EINVAL;
+
+	if ((file->f_mode & FMODE_WRITE) && (!file->f_op->write_iter))
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * loop_change_fd switched the backing store of a loopback device to
  * a new file. This is useful for operating system installers to free up
@@ -526,6 +537,10 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 	if (!file)
 		return -EBADF;
 
+	error = loop_check_backing_file(file);
+	if (error)
+		return error;
+
 	/* suppress uevents while reconfiguring the device */
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
 
@@ -963,6 +978,14 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 
 	if (!file)
 		return -EBADF;
+
+	if ((mode & BLK_OPEN_WRITE) && (!file->f_op->write_iter))
+		return -EINVAL;
+
+	error = loop_check_backing_file(file);
+	if (error)
+		return error;
+
 	is_loop = is_loop_device(file);
 
 	/* This is safe, since we have a reference from open(). */
-- 
2.43.0
Re: [PATCH V4] loop: Add sanity check for read/write_iter
Posted by Christoph Hellwig 7 months, 3 weeks ago 
On Mon, Apr 28, 2025 at 10:15:44PM +0800, Lizhi Xu wrote:
> +	if ((file->f_mode & FMODE_WRITE) && (!file->f_op->write_iter))

No need for the braces around !file->f_op->write_iter.

> +	if ((mode & BLK_OPEN_WRITE) && (!file->f_op->write_iter))

Same here.  Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
[PATCH V5] loop: Add sanity check for read/write_iter
Posted by Lizhi Xu 7 months, 3 weeks ago 
Some file systems do not support read_iter/write_iter, such as selinuxfs
in this issue.
So before calling them, first confirm that the interface is supported and
then call it.

It is releavant in that vfs_iter_read/write have the check, and removal
of their used caused szybot to be able to hit this issue.

Fixes: f2fed441c69b ("loop: stop using vfs_iter__{read,write} for buffered I/O")
Reported-by: syzbot+6af973a3b8dfd2faefdc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6af973a3b8dfd2faefdc
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
V1 -> V2: move check to loop_configure and loop_change_fd
V2 -> V3: using helper for this check
V3 -> V4: remove input parameters change and mode
V4 -> V5: remove braces around !file->f_op->write_iter

 drivers/block/loop.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 46cba261075f..655d33e63cb9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -505,6 +505,17 @@ static void loop_assign_backing_file(struct loop_device *lo, struct file *file)
 	lo->lo_min_dio_size = loop_query_min_dio_size(lo);
 }
 
+static int loop_check_backing_file(struct file *file)
+{
+	if (!file->f_op->read_iter)
+		return -EINVAL;
+
+	if ((file->f_mode & FMODE_WRITE) && !file->f_op->write_iter)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * loop_change_fd switched the backing store of a loopback device to
  * a new file. This is useful for operating system installers to free up
@@ -526,6 +537,10 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 	if (!file)
 		return -EBADF;
 
+	error = loop_check_backing_file(file);
+	if (error)
+		return error;
+
 	/* suppress uevents while reconfiguring the device */
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
 
@@ -963,6 +978,14 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 
 	if (!file)
 		return -EBADF;
+
+	if ((mode & BLK_OPEN_WRITE) && !file->f_op->write_iter)
+		return -EINVAL;
+
+	error = loop_check_backing_file(file);
+	if (error)
+		return error;
+
 	is_loop = is_loop_device(file);
 
 	/* This is safe, since we have a reference from open(). */
-- 
2.43.0
Re: [PATCH V5] loop: Add sanity check for read/write_iter
Posted by Christian Hesse 7 months ago 
Lizhi Xu <lizhi.xu@windriver.com> on Mon, 2025/04/28 22:36:
> Some file systems do not support read_iter/write_iter, such as selinuxfs
> in this issue.
> So before calling them, first confirm that the interface is supported and
> then call it.
> 
> It is releavant in that vfs_iter_read/write have the check, and removal
> of their used caused szybot to be able to hit this issue.
> 
> Fixes: f2fed441c69b ("loop: stop using vfs_iter__{read,write} for buffered
> I/O") Reported-by: syzbot+6af973a3b8dfd2faefdc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6af973a3b8dfd2faefdc
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> V1 -> V2: move check to loop_configure and loop_change_fd
> V2 -> V3: using helper for this check
> V3 -> V4: remove input parameters change and mode
> V4 -> V5: remove braces around !file->f_op->write_iter

This introduced a regression for Arch Linux, breaking boot media generated
with archiso [0]. More specifically it's this call of losetup [1].

There's a squashfs inside iso9660. Mounting the iso9660 filesystem works
fine, but losetup complains when setting up:

$ losetup --find --show --read-only -- /run/archiso/bootmnt/arch/x86_64/airootfs.sfs
losetup: /run/archiso/bootmnt/arch/x86_64/airootfs.sfs: failed to set up loop device: Invalid argument

This has been bisected to commit d278164832618bf2775c6a89e6434e2633de1eed in
mainline (and 9bd3feb324fce2e93e09d0a5b00887e81d337a8c for linux-6.14.y,
184b147b9f7f07577567a80fcc9314f2bd0b0b00 for linux-6.12.y). Thanks to
Christian Heusel for his work on this.

As the call tries to setup in read-only mode the check for
(file->f_op->read_iter) fails here, returning the -EINVAL we see.

Reported-by: Christian Hesse <mail@eworm.de>
Bisected-by: Christian Heusel <christian@heusel.eu>

[0] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio-archiso
[1] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio-archiso/-/blob/master/hooks/archiso?ref_type=heads#L88
-- 
Mit freundlichen Gruessen
Christian Hesse
Re: [PATCH V5] loop: Add sanity check for read/write_iter
Posted by Lizhi Xu 7 months ago 
On Mon, 19 May 2025 17:56:40 +0200, Christian Hesse wrote:
> $ losetup --find --show --read-only -- /run/archiso/bootmnt/arch/x86_64/airootfs.sfs
> losetup: /run/archiso/bootmnt/arch/x86_64/airootfs.sfs: failed to set up loop device: Invalid argument
I tried to reproduce the problem you mentioned using the kernel containing
"commit:f5c84eff", but failed to reproduce it.
The complete reproduction steps are as follows:

sudo apt install squashfs-tools debootstrap
sudo debootstrap --arch=amd64 focal rootfs http://archive.ubuntu.com/ubuntu/
sudo mksquashfs rootfs rootfs.sfs -comp xz -e boot
root@intel-x86-64:~# losetup --find --show --read-only -- rootfs.sfs
[   79.676267][ T9551] loop0: detected capacity change from 0 to 272400
/dev/loop0
root@intel-x86-64:~# uname -a
Linux intel-x86-64 6.15.0-rc7 #108 SMP PREEMPT_DYNAMIC Mon May 19 09:20:25 CST 2025 x86_64 x86_64 x86_64 GNU/Linux
root@intel-x86-64:~# df -Th
Filesystem     Type      Size  Used Avail Use% Mounted on
/dev/root      ext4      3.8G  738M  2.9G  21% /
devtmpfs       devtmpfs  1.5G     0  1.5G   0% /dev
tmpfs          tmpfs     1.5G     0  1.5G   0% /dev/shm
tmpfs          tmpfs     585M   13M  572M   3% /run
tmpfs          tmpfs     4.0M     0  4.0M   0% /sys/fs/cgroup
tmpfs          tmpfs     1.5G     0  1.5G   0% /tmp
tmpfs          tmpfs     1.5G  904K  1.5G   1% /var/volatile
tmpfs          tmpfs     293M     0  293M   0% /run/user/0
root@intel-x86-64:~# ls -lath /dev/loop0
brw-rw---- 1 root disk 7, 0 May 20 02:43 /dev/loop0
root@intel-x86-64:~# mkdir sfs
root@intel-x86-64:~# mount /dev/loop0 sfs
mount: /root/sfs: WARNING: source write-protected, mounted read-only.
root@intel-x86-64:~# df -Th
Filesystem     Type      Size  Used Avail Use% Mounted on
/dev/root      ext4      3.8G  738M  2.9G  21% /
devtmpfs       devtmpfs  1.5G     0  1.5G   0% /dev
tmpfs          tmpfs     1.5G     0  1.5G   0% /dev/shm
tmpfs          tmpfs     585M   13M  572M   3% /run
tmpfs          tmpfs     4.0M     0  4.0M   0% /sys/fs/cgroup
tmpfs          tmpfs     1.5G     0  1.5G   0% /tmp
tmpfs          tmpfs     1.5G  904K  1.5G   1% /var/volatile
tmpfs          tmpfs     293M     0  293M   0% /run/user/0
/dev/loop0     squashfs  134M  134M     0 100% /root/sfs
root@intel-x86-64:~# ls -alt sfs
total 3
drwx------ 21 root root 3072 May 20 02:49 ..
drwxr-xr-x 16 root root  284 May 20 02:20 .
drwxrwxrwt  2 root root    3 May 20 02:20 tmp
drwxr-xr-x 59 root root 2073 May 20 02:20 etc
drwxr-xr-x  8 root root  124 May 20 02:20 run
drwxr-xr-x  2 root root    3 May 20 02:19 media
drwxr-xr-x  2 root root    3 May 20 02:19 mnt
drwxr-xr-x  2 root root    3 May 20 02:19 opt
drwx------  2 root root   46 May 20 02:19 root
drwxr-xr-x  2 root root    3 May 20 02:19 srv
drwxr-xr-x 13 root root  178 May 20 02:19 usr
drwxr-xr-x 11 root root  172 May 20 02:19 var
drwxr-xr-x  4 root root  191 May 20 02:19 dev
lrwxrwxrwx  1 root root    7 May 20 02:19 bin -> usr/bin
lrwxrwxrwx  1 root root    7 May 20 02:19 lib -> usr/lib
lrwxrwxrwx  1 root root    9 May 20 02:19 lib32 -> usr/lib32
lrwxrwxrwx  1 root root    9 May 20 02:19 lib64 -> usr/lib64
lrwxrwxrwx  1 root root   10 May 20 02:19 libx32 -> usr/libx32
lrwxrwxrwx  1 root root    8 May 20 02:19 sbin -> usr/sbin
drwxr-xr-x  2 root root    3 Apr 15  2020 home
drwxr-xr-x  2 root root    3 Apr 15  2020 proc
drwxr-xr-x  2 root root    3 Apr 15  2020 sys
Re: [PATCH V5] loop: Add sanity check for read/write_iter
Posted by Christian Hesse 7 months ago 
Lizhi Xu <lizhi.xu@windriver.com> on Tue, 2025/05/20 11:00:
> On Mon, 19 May 2025 17:56:40 +0200, Christian Hesse wrote:
> > $ losetup --find --show --read-only --
> > /run/archiso/bootmnt/arch/x86_64/airootfs.sfs losetup:
> > /run/archiso/bootmnt/arch/x86_64/airootfs.sfs: failed to set up loop
> > device: Invalid argument
>
> I tried to reproduce the problem you mentioned using the kernel containing
> "commit:f5c84eff", but failed to reproduce it.
> The complete reproduction steps are as follows:
> 
> sudo apt install squashfs-tools debootstrap
> sudo debootstrap --arch=amd64 focal rootfs http://archive.ubuntu.com/ubuntu/
> sudo mksquashfs rootfs rootfs.sfs -comp xz -e boot
> [...]

That's the wrong end of the stack. After all squashfs is not directly
involved here (that was just an etxra info on why we have a loopback file
inside iso9660).

The issue is setting up the loopback file inside a mounted iso9660 filesystem.
Take these steps for easy reproduction:

root@leda ~ # mkdir iso.d 
root@leda ~ # truncate -s 10m iso.d/loopback.img
root@leda ~ # mkisofs -o iso.iso iso.d/
Setting input-charset to 'UTF-8' from locale.
 94,75% done, estimate finish Tue May 20 07:34:52 2025
Total translation table size: 0
Total rockridge attributes bytes: 0
Total directory bytes: 0
Path table size(bytes): 10
Max brk space used 0
5294 extents written (10 MB)
root@leda ~ # mount -o loop iso.iso /mnt/tmp 
mount: /mnt/tmp: WARNING: source write-protected, mounted read-only.
root@leda ~ # losetup --find --show --read-only -- /mnt/tmp/loopback.img 
losetup: /mnt/tmp/loopback.img: failed to set up loop device: Invalid argument

Hope that helps, let me know if you need more assistance.
-- 
Best regards,
Chris
Re: [PATCH V5] loop: Add sanity check for read/write_iter
Posted by Jens Axboe 7 months, 2 weeks ago 
On Mon, 28 Apr 2025 22:36:26 +0800, Lizhi Xu wrote:
> Some file systems do not support read_iter/write_iter, such as selinuxfs
> in this issue.
> So before calling them, first confirm that the interface is supported and
> then call it.
> 
> It is releavant in that vfs_iter_read/write have the check, and removal
> of their used caused szybot to be able to hit this issue.
> 
> [...]

Applied, thanks!

[1/1] loop: Add sanity check for read/write_iter
      commit: f5c84eff634ba003326aa034c414e2a9dcb7c6a7

Best regards,
-- 
Jens Axboe

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.