Re: [PATCH] floppy: refactor open() flags handling (was Re: mm: uninterruptable tasks hanged on mmap_sem)

[Dmitry Vyukov <dvyukov@google.com>]

On Thu, Feb 4, 2016 at 10:22 PM, Jiri Kosina <jikos@kernel.org> wrote:
> On Tue, 2 Feb 2016, Dmitry Vyukov wrote:
>
>> If the following program run in a parallel loop, eventually it leaves
>> hanged uninterruptable tasks on mmap_sem.
>>
>> [ 4074.740298] sysrq: SysRq : Show Locks Held
>> [ 4074.740780] Showing all locks held in the system:
>> ...
>> [ 4074.762133] 1 lock held by a.out/1276:
>> [ 4074.762427]  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff816df89c>]
>> __mm_populate+0x25c/0x350
>> [ 4074.763149] 1 lock held by a.out/1147:
>> [ 4074.763438]  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff816b3bbc>]
>> vm_mmap_pgoff+0x12c/0x1b0
>> [ 4074.764164] 1 lock held by a.out/1284:
>> [ 4074.764447]  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff816df89c>]
>> __mm_populate+0x25c/0x350
>> [ 4074.765287]
>>
>> They all look as follows:
>>
>> # cat /proc/1284/task/**/stack
>> [<ffffffff82c14d13>] call_rwsem_down_write_failed+0x13/0x20
>> [<ffffffff816b3bbc>] vm_mmap_pgoff+0x12c/0x1b0
>> [<ffffffff81700c58>] SyS_mmap_pgoff+0x208/0x580
>> [<ffffffff811aeeb6>] SyS_mmap+0x16/0x20
>> [<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a
>> [<ffffffffffffffff>] 0xffffffffffffffff
>> [<ffffffff8164893e>] wait_on_page_bit+0x1de/0x210
>> [<ffffffff8165572b>] filemap_fault+0xfeb/0x14d0
>> [<ffffffff816e1972>] __do_fault+0x1b2/0x3e0
>> [<ffffffff816f080e>] handle_mm_fault+0x1b4e/0x49a0
>> [<ffffffff816ddae0>] __get_user_pages+0x2c0/0x11a0
>> [<ffffffff816df5a8>] populate_vma_page_range+0x198/0x230
>> [<ffffffff816df83b>] __mm_populate+0x1fb/0x350
>> [<ffffffff816f90c1>] do_mlock+0x291/0x360
>> [<ffffffff816f962b>] SyS_mlock2+0x4b/0x70
>> [<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a
>> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Dmitry,
>
> could you please feed the patch below (on top of the previous floppy fix)
> to your syzkaller machinery and test whether you are still able to
> reproduce the problem? It passess my local testing here.


Now that open exits early with EWOULDBLOCK, I guess the reproduced is
not doing anything particularly interesting. But FWIW it fixes the
crash for me :)




> Thanks!
>
>
>
>
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] floppy: refactor open() flags handling
>
> In case /dev/fdX is open with O_NDELAY / O_NONBLOCK, floppy_open() immediately
> succeeds, without performing any further media / controller preparations.
> That's "correct" wrt. the NODELAY flag, but is hardly correct wrt. the rest
> of the floppy driver, that is not really O_NONBLOCK ready, at all. Therefore
> it's not too surprising, that subsequent attempts to work with the
> filedescriptor produce bad results. Namely, syzkaller tool has been able
> to livelock mmap() on the returned fd to keep waiting on the page unlock
> bit forever.
>
> Fortunately, POSIX allows us to not support non-blocking behavior on fdX
> block device.
>
> Quite frankly, I'd have trouble defining what non-blocking behavior would
> be for floppies. Is waiting ages for the driver to actually succeed
> reading a sector blocking operation? Is waiting for drive motor to start
> blocking operation? How about in case of virtualized floppies?
>
> Given the fact that POSIX allows us to not support non-blocking behavior,
> and given the fact that such behavior would be difficult to define anyway,
> change the handling of FMODE_NDELAY in floppy_open() so that it returns
> EWOULDBLOCK.
>
> While at it, clean up a bit handling of !(mode & (FMODE_READ|FMODE_WRITE))
> case and return EINVAL instead of succeeding as well.
>
> Spotted by syzkaller tool.
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> NOT-YET-Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  drivers/block/floppy.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index b206115..50faf7f 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -3663,6 +3663,15 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
>
>         opened_bdev[drive] = bdev;
>
> +       if (mode & FMODE_NDELAY) {
> +               res = -EWOULDBLOCK;
> +               goto out;
> +       }
> +       if (!(mode & (FMODE_READ|FMODE_WRITE))) {
> +               res = -EINVAL;
> +               goto out;
> +       }
> +
>         res = -ENXIO;
>
>         if (!floppy_track_buffer) {
> @@ -3706,21 +3715,20 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
>         if (UFDCS->rawcmd == 1)
>                 UFDCS->rawcmd = 2;
>
> -       if (!(mode & FMODE_NDELAY)) {
> -               if (mode & (FMODE_READ|FMODE_WRITE)) {
> -                       UDRS->last_checked = 0;
> -                       clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags);
> -                       check_disk_change(bdev);
> -                       if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags))
> -                               goto out;
> -                       if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags))
> -                               goto out;
> -               }
> -               res = -EROFS;
> -               if ((mode & FMODE_WRITE) &&
> -                   !test_bit(FD_DISK_WRITABLE_BIT, &UDRS->flags))
> -                       goto out;
> -       }
> +       UDRS->last_checked = 0;
> +       clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags);
> +       check_disk_change(bdev);
> +       if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags))
> +               goto out;
> +       if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &UDRS->flags))
> +               goto out;
> +
> +       res = -EROFS;
> +
> +       if ((mode & FMODE_WRITE) &&
> +                       !test_bit(FD_DISK_WRITABLE_BIT, &UDRS->flags))
> +               goto out;
> +
>         mutex_unlock(&open_lock);
>         mutex_unlock(&floppy_mutex);
>         return 0;
>
> --
> Jiri Kosina
> SUSE Labs
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>