On Thu 16-05-19 14:44:07, Jens Axboe wrote:
> On 5/16/19 8:01 AM, Jan Kara wrote:
> > Loop module allows calling LOOP_SET_FD while there are other openers of
> > the loop device. Even exclusive ones. This can lead to weird
> > consequences such as kernel deadlocks like:
> >
> > mount_bdev() lo_ioctl()
> > udf_fill_super()
> > udf_load_vrs()
> > sb_set_blocksize() - sets desired block size B
> > udf_tread()
> > sb_bread()
> > __bread_gfp(bdev, block, B)
> > loop_set_fd()
> > set_blocksize()
> > - now __getblk_slow() indefinitely loops because B != bdev
> > block size
> >
> > Fix the problem by disallowing LOOP_SET_FD ioctl when there are
> > exclusive openers of a loop device.
> >
> > [Deliberately chosen not to CC stable as a user with priviledges to
> > trigger this race has other means of taking the system down and this
> > has a potential of breaking some weird userspace setup]
> >
> > Reported-and-tested-by:
> > [email protected]
> > Signed-off-by: Jan Kara <[email protected]>
> > ---
> > drivers/block/loop.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > Hi Jens!
> >
> > What do you think about this patch? It fixes the problem but it also
> > changes user visible behavior so there are chances it breaks some
> > existing setup (although I have hard time coming up with a realistic
> > scenario where it would matter).
>
> I also have a hard time thinking about valid cases where this would be a
> problem. I think, in the end, that fixing the issue is more important
> than a potentially hypothetical use case.
>
> > Alternatively we could change getblk() code handle changing block
> > size. That would fix the particular issue syzkaller found as well but
> > I'm not sure what else is broken when block device changes while fs
> > driver is working with it.
>
> I think your solution here is saner.
Will you pick up the patch please? I cannot find it in your tree... Thanks!
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
