On Tue 10-11-26 10:14:50, Christian Brauner wrote: > Currently loop accepts any regular file that implements ->read_iter() as > a backing file and any that also implements ->write_iter() as a writable > one. That includes files on procfs, sysfs and other pseudo filesystems. > For example, /proc/kcore attaches read-only as a loop device the size of > the kernel's virtual address space, 64 PiB with 5-level paging. sysfs > attributes attach read-write as 4096 byte loop devices. Writing a block > to the loop device of /sys/kernel/mm/transparent_hugepage/enabled > changes the THP setting. > > That's not just nonsensical, it can be abused. Loop calls ->read_iter() > and ->write_iter() of the backing file directly from its worker which > runs with kernel credentials. But pseudo files enforce part of their > access rules at read and write time against current. Dozens of sysfs > ->show() and ->store() methods call capable(). Behind a loop device all > of these checks are done against the loop worker instead of the task > that opened the file. And setting up a loop device requires no > capabilities, only access to the device nodes. So on v7.4-rc1: > > - uid 0 without any capabilities can't write to /sys/kernel/reboot/mode > as mode_store() requires CAP_SYS_BOOT. Writing "warm" through a loop > device works. > > - uid 0 without any capabilities can't read the files in > /sys/firmware/dmi/entries/*/ as dmi_sysfs_attr_show() requires > CAP_SYS_ADMIN. Reading them through a loop device works. > > - An unprivileged container that was handed a loop device can't change > gro_flush_timeout of its own network devices as > gro_flush_timeout_store() requires CAP_NET_ADMIN in the initial user > namespace. Writing it through a loop device works. > > The same goes for hard_offline_page, the md and sd attributes, rfkill > and others. > > Refuse files on pseudo filesystems as loop backing files. Note that > file_is_pseudo() looks through overlayfs so stacking overlayfs on top of > sysfs doesn't get around this. > > Signed-off-by: Christian Brauner (Amutable) <[email protected]>
Looks good. Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > --- > drivers/block/loop.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index c96e35c3ef42..1482580a1979 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -537,6 +537,9 @@ static int loop_check_backing_file(struct file *file) > if ((file->f_mode & FMODE_WRITE) && !file->f_op->write_iter) > return -EINVAL; > > + if (file_is_pseudo(file)) > + return -EINVAL; > + > return 0; > } > > > -- > 2.53.0 > -- Jan Kara <[email protected]> SUSE Labs, CR
