mm: uninterruptable tasks hanged on mmap_sem

[Dmitry Vyukov]

Hello,

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

# cat /proc/1284/status
Name: a.out
State: D (disk sleep)
Tgid: 1147
Ngid: 0
Pid: 1284
PPid: 28436
TracerPid: 0
Uid: 0 0 0 0
Gid: 0 0 0 0
FDSize: 64
Groups: 0
NStgid: 1147
NSpid: 1284
NSpgid: 28436
NSsid: 6529
VmPeak: 50356 kB
VmSize: 50356 kB
VmLck: 16 kB
VmPin: 0 kB
VmHWM: 8 kB
VmRSS: 8 kB
RssAnon: 8 kB
RssFile: 0 kB
RssShmem: 0 kB
VmData: 49348 kB
VmStk: 136 kB
VmExe: 828 kB
VmLib: 8 kB
VmPTE: 44 kB
VmPMD: 12 kB
VmSwap: 0 kB
HugetlbPages: 0 kB
Threads: 2
SigQ: 1/3189
SigPnd: 0000000000000100
ShdPnd: 0000000000000100
SigBlk: 0000000000000000
SigIgn: 0000000000000000
SigCgt: 0000000180000000
CapInh: 0000000000000000
CapPrm: 0000003fffffffff
CapEff: 0000003fffffffff
CapBnd: 0000003fffffffff
CapAmb: 0000000000000000
Seccomp: 0
Cpus_allowed: f
Cpus_allowed_list: 0-3
Mems_allowed: 00000000,00000003
Mems_allowed_list: 0-1
voluntary_ctxt_switches: 3
nonvoluntary_ctxt_switches: 1


There are no BUGs, WARNINGs, stalls on console.

Not sure if its mm or floppy fault.


// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <pthread.h>
#include <stdint.h>
#include <string.h>
#include <sys/syscall.h>
#include <unistd.h>

#ifndef SYS_mlock2
#define SYS_mlock2 325
#endif

long r[7];

void* thr(void* arg)
{
switch ((long)arg) {
case 0:
r[0] = syscall(SYS_mmap, 0x20000000ul, 0x1000ul, 0x3ul, 0x32ul,
0xfffffffffffffffful, 0x0ul);
break;
case 1:
memcpy((void*)0x20000000, "\x2f\x64\x65\x76\x2f\x66\x64\x23", 8);
r[2] = syscall(SYS_open, "/dev/fd0", 0x800ul, 0, 0, 0);
break;
case 2:
r[3] = syscall(SYS_mmap, 0x20001000ul, 0x1000ul, 0x3ul, 0x32ul,
0xfffffffffffffffful, 0x0ul);
break;
case 3:
r[4] = syscall(SYS_mmap, 0x20002000ul, 0x1000ul, 0x3ul, 0x812ul,
r[2], 0x0ul);
break;
case 4:
r[5] = syscall(SYS_mmap, 0x20003000ul, 0x1000ul, 0x3ul, 0x32ul,
0xfffffffffffffffful, 0x0ul);
break;
case 5:
r[6] = syscall(SYS_mlock2, 0x20000000ul, 0x4000ul, 0x0ul, 0, 0, 0);
break;
}
return 0;
}

int main()
{
long i;
pthread_t th[6];

srand(getpid());
memset(r, -1, sizeof(r));
for (i = 0; i < 6; i++) {
pthread_create(&th[i], 0, thr, (void*)i);
usleep(10000);
}
for (i = 0; i < 6; i++) {
pthread_create(&th[i], 0, thr, (void*)i);
if (rand() % 2)
usleep(rand() % 10000);
}
usleep(100000);
return 0;
}

On commit 36f90b0a2ddd60823fe193a85e60ff1906c2a9b3.

[Jiri Kosina]

This stacktrace is odd.

<joke, but not really>I am pretty sure that it's floppy fault,
even before I looked at the reproducer</joke>

>
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <pthread.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
>
> #ifndef SYS_mlock2
> #define SYS_mlock2 325
> #endif
>
> long r[7];
>
> void* thr(void* arg)
> {
> switch ((long)arg) {
> case 0:
> r[0] = syscall(SYS_mmap, 0x20000000ul, 0x1000ul, 0x3ul, 0x32ul,
> 0xfffffffffffffffful, 0x0ul);
> break;
> case 1:
> memcpy((void*)0x20000000, "\x2f\x64\x65\x76\x2f\x66\x64\x23", 8);
> r[2] = syscall(SYS_open, "/dev/fd0", 0x800ul, 0, 0, 0);

Just to make sure -- I guess that this is a minimal testcase already,
right? IOW, if you eliminate the open(/dev/fd0) call, the bug will vanish?

I'll try to reproduce this later tonight or tomorrow.

--
Jiri Kosina
SUSE Labs

[Dmitry Vyukov]

Here it is with line numbers and inlined frames if it helps:

[<ffffffff82c14d13>] call_rwsem_down_write_failed+0x13/0x20
arch/x86/lib/rwsem.S:99
[<ffffffff816b3bbc>] vm_mmap_pgoff+0x12c/0x1b0 mm/util.c:327
[< inline >] SYSC_mmap_pgoff mm/mmap.c:1453
[<ffffffff81700c58>] SyS_mmap_pgoff+0x208/0x580 mm/mmap.c:1411
[< inline >] SYSC_mmap arch/x86/kernel/sys_x86_64.c:95
[<ffffffff811aeeb6>] SyS_mmap+0x16/0x20 arch/x86/kernel/sys_x86_64.c:86
[<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

[<ffffffff8164893e>] wait_on_page_bit+0x1de/0x210 mm/filemap.c:762
[< inline >] wait_on_page_locked include/linux/pagemap.h:526
[<ffffffff8165572b>] filemap_fault+0xfeb/0x14d0 mm/filemap.c:2118
[<ffffffff816e1972>] __do_fault+0x1b2/0x3e0 mm/memory.c:2778
[< inline >] do_cow_fault mm/memory.c:3008
[< inline >] do_fault mm/memory.c:3136
[< inline >] handle_pte_fault mm/memory.c:3308
[< inline >] __handle_mm_fault mm/memory.c:3418
[<ffffffff816f080e>] handle_mm_fault+0x1b4e/0x49a0 mm/memory.c:3447
[< inline >] faultin_page mm/gup.c:375
[<ffffffff816ddae0>] __get_user_pages+0x2c0/0x11a0 mm/gup.c:568
[<ffffffff816df5a8>] populate_vma_page_range+0x198/0x230 mm/gup.c:992
[<ffffffff816df83b>] __mm_populate+0x1fb/0x350 mm/gup.c:1042
[<ffffffff816f90c1>] do_mlock+0x291/0x360 mm/mlock.c:650
[< inline >] SYSC_mlock2 mm/mlock.c:671
[<ffffffff816f962b>] SyS_mlock2+0x4b/0x70 mm/mlock.c:661
[<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

I have not tried to remove it, but my gut feeling says that it is necessary :)

[Dmitry Vyukov]

On Tue, Feb 2, 2016 at 10:14 PM, Dmitry Vyukov <dvy...@google.com> wrote:
> On Tue, Feb 2, 2016 at 10:08 PM, Jiri Kosina <ji...@kernel.org> wrote:
>> On Tue, 2 Feb 2016, Dmitry Vyukov wrote:
>>
>>> Hello,
>>>
>>> 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]

Original log from fuzzer contained the following WARNING in
mm/rmap.c:412. But when I tried to reproduce it, I hit these hanged
processes instead. I can't reliably detect what program triggered
what. So it may be related, or maybe a separate issue.

------------[ cut here ]------------
kernel BUG at mm/rmap.c:412!
invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
Modules linked in:
CPU: 2 PID: 20110 Comm: udevd Tainted: G W 4.5.0-rc2+ #306
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff880031c32f80 ti: ffff8800376e0000 task.ti: ffff8800376e0000
RIP: 0010:[<ffffffff81715dd7>] [<ffffffff81715dd7>]
unlink_anon_vmas+0x407/0x600
RSP: 0018:ffff8800376e7ba0 EFLAGS: 00010297
RAX: ffff880031c32f80 RBX: ffff88003d81f448 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88003e39408c
RBP: ffff8800376e7be8 R08: ffff880034dc35e8 R09: 00000001001d001a
R10: ffff880031c32f80 R11: 0000000000000000 R12: ffff880034dc2908
R13: ffff880034dc2908 R14: ffff880034dc28f8 R15: ffff88003e394000
FS: 0000000000000000(0000) GS:ffff88006d600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00007f12dbfbe140 CR3: 000000003552a000 CR4: 00000000000006e0
Stack:
ffff88003d81f3e0 ffff88003d81f458 ffff88003d81f458 ffff88005e516580
ffff880039e1df20 ffff88003d81f3e0 dffffc0000000000 00007f12dc802000
ffff88003d81f430 ffff8800376e7c48 ffffffff816e50ad 0000000000000000
Call Trace:
[<ffffffff816e50ad>] free_pgtables+0x1bd/0x3b0 mm/memory.c:555
[<ffffffff81703613>] exit_mmap+0x233/0x410 mm/mmap.c:2850
[<ffffffff8134bf05>] mmput+0x95/0x230 kernel/fork.c:706
[< inline >] exit_mm kernel/exit.c:436
[<ffffffff8135e3b2>] do_exit+0x7b2/0x2cb0 kernel/exit.c:735
[<ffffffff81360a28>] do_group_exit+0x108/0x330 kernel/exit.c:878
[< inline >] SYSC_exit_group kernel/exit.c:889
[<ffffffff81360c6d>] SyS_exit_group+0x1d/0x20 kernel/exit.c:887
[<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
Code: 07 83 c2 03 38 ca 7c 08 84 c9 0f 85 9e 01 00 00 41 8b 87 8c 00
00 00 49 89 de 48 8b 5d d0 85 c0 0f 84 43 ff ff ff e8 d9 5d e5 ff <0f>
0b e8 d2 5d e5 ff 4c 89 ff e8 fa e5 ff ff e9 42 ff ff ff e8
RIP [<ffffffff81715dd7>] unlink_anon_vmas+0x407/0x600 mm/rmap.c:412
RSP <ffff8800376e7ba0>
---[ end trace 5282279c07ce8f67 ]---
Fixing recursive fault but reboot is needed!
floppy0: disk absent or changed during operation
blk_update_request: I/O error, dev fd0, sector 0
floppy0: disk absent or changed during operation
blk_update_request: I/O error, dev fd0, sector 0

[Jiri Kosina]

On Tue, 2 Feb 2016, Dmitry Vyukov wrote:

> Original log from fuzzer contained the following WARNING in
> mm/rmap.c:412. But when I tried to reproduce it, I hit these hanged
> processes instead. I can't reliably detect what program triggered
> what. So it may be related, or maybe a separate issue.
>
> ------------[ cut here ]------------
> kernel BUG at mm/rmap.c:412!

Are you by any chance in this test sending signals to the fuzzer?

If so, the bug I just fixed in floppy driver can cause all kinds of memory
corruptions in case you're running multithreaded accessess to /dev/fd0 and
sending singals to the threads that are trying to access /dev/fd0 at the
same time.

Could you please double check that the other floppy fix I've sent you a
couple days ago doesn't fix this as well? (this test makes sense only if
signals are involved though).

Thanks,

[Dmitry Vyukov]

I have "floppy: fix lock_fdc() signal handling" applied (the final,
second version).
The process is multithreaded and it can well receive SIGKILLs.

[Jiri Kosina]

On Tue, 2 Feb 2016, Dmitry Vyukov wrote:

> Hello,
>
> 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]

I've now tried to reproduce this on 4.5-rc1 (with the lock_fdc() fix
applied), and I am not seeing any stuck tasks so far.

Could you please provide more details about the reproduction scenario?
Namely, how many parallel invocations are you typically running (and how
many cores does the system in question have), and what is the typical time
that you need for the problem to appear?

[Dmitry Vyukov]

Qemu with 4 cores, 32 parallel processes, it took 20 seconds (2409
program executions) to hang 2 of them just now.

[Jiri Kosina]

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.

I am now able to reproduce the issue and will be looking into it.

[ .. snip .. ]

> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <pthread.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
>
> #ifndef SYS_mlock2
> #define SYS_mlock2 325
> #endif
>
> long r[7];
>
> void* thr(void* arg)
> {
> switch ((long)arg) {
> case 0:
> r[0] = syscall(SYS_mmap, 0x20000000ul, 0x1000ul, 0x3ul, 0x32ul,
> 0xfffffffffffffffful, 0x0ul);
> break;
> case 1:
> memcpy((void*)0x20000000, "\x2f\x64\x65\x76\x2f\x66\x64\x23", 8);

The memcpy() can be removed and the problem still triggers reliably for
me.

> case 3:
> r[4] = syscall(SYS_mmap, 0x20002000ul, 0x1000ul, 0x3ul, 0x812ul,
> r[2], 0x0ul);

0x812 made me wonder (it's not really meaningful flags value), but the bug
triggers also with 0x12.

[Jiri Kosina]

On Tue, 2 Feb 2016, Dmitry Vyukov wrote:

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.

Thanks!




From: Jiri Kosina <jko...@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 <dvy...@google.com>
NOT-YET-Signed-off-by: Jiri Kosina <jko...@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;

[Dmitry Vyukov]

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 :)

[Jiri Kosina]

On Fri, 5 Feb 2016, Dmitry Vyukov wrote:

> > 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.

Yeah. But as I explained in the changelog, I think it's a valid thing to
do (opinions welcome).

I don't think having a huge discussion about what nonblocking really means
for floppy and then try to refactor the whole driver to support that would
make sense.

Alternatively we can take more conservative aproach, accept the
nonblocking flag, but do the regular business of the driver.

Actually, let's try that, to make sure that we don't introduce userspace
breakage.

Could you please retest with the patch below?

Thanks a lot.

From: Jiri Kosina <jko...@suse.cz>
Subject: [PATCH v2] 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.

Quite frankly, I 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?

One option would be returning EWOULDBLOCK in case O_NDLEAY / O_NONBLOCK is
being passed to open(). That has a theoretical potential of breaking some
arcane and archaic userspace though.

Let's take a more conservative aproach, and accept the O_NDLEAY flag, and let
the driver behave as usual.

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 <dvy...@google.com>
NOT-YET-Signed-off-by: Jiri Kosina <jko...@suse.cz>
---

drivers/block/floppy.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index d15d415..f7d4d7b 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3662,6 +3662,11 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)

opened_bdev[drive] = bdev;

+ if (!(mode & (FMODE_READ|FMODE_WRITE))) {
+ res = -EINVAL;
+ goto out;
+ }
+
res = -ENXIO;

if (!floppy_track_buffer) {

@@ -3705,21 +3710,20 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)

[Dmitry Vyukov]

On Fri, Feb 5, 2016 at 2:51 PM, Jiri Kosina <ji...@kernel.org> wrote:
> On Fri, 5 Feb 2016, Dmitry Vyukov wrote:
>
>> > 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.
>
> Yeah. But as I explained in the changelog, I think it's a valid thing to
> do (opinions welcome).
>
> I don't think having a huge discussion about what nonblocking really means
> for floppy and then try to refactor the whole driver to support that would
> make sense.

I don't have any objections. And I agree that it does not make sense
to spend any considerable time on optimizing this driver.

> Alternatively we can take more conservative aproach, accept the
> nonblocking flag, but do the regular business of the driver.
>
> Actually, let's try that, to make sure that we don't introduce userspace
> breakage.
>
> Could you please retest with the patch below?

Reapplied.
Agree that it's better to not bail out on O_NONBLOCK.

[Jiri Kosina]

On Fri, 5 Feb 2016, Dmitry Vyukov wrote:

> I don't have any objections. And I agree that it does not make sense
> to spend any considerable time on optimizing this driver.

Yeah, on a second thought this definitely is the way how to deal with this
in this particular driver.

> > Alternatively we can take more conservative aproach, accept the
> > nonblocking flag, but do the regular business of the driver.
> >
> > Actually, let's try that, to make sure that we don't introduce userspace
> > breakage.
> >
> > Could you please retest with the patch below?
>
> Reapplied.

Thanks. Once/if you confirm that syzkaller is not able to reproduce the
problem any more, I'll queue it and push to Jens.

[Dmitry Vyukov]

Tested. Fixes the hang for me.