| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACT4Y+aBCt_pVK+SY9fRpRFU9KTVOChn_vs5pv_KFiUbkGCm4Q@mail.gmail.com>
Date: Fri, 5 Feb 2016 14:43:35 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Jiri Kosina <jikos@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Oleg Nesterov <oleg@...hat.com>,
Konstantin Khlebnikov <koct9i@...il.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>,
Takashi Iwai <tiwai@...e.de>,
syzkaller <syzkaller@...glegroups.com>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [PATCH] floppy: refactor open() flags handling (was Re: mm:
uninterruptable tasks hanged on mmap_sem)
On Thu, Feb 4, 2016 at 10:22 PM, Jiri Kosina <jikos@...nel.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@...e.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@...gle.com>
> NOT-YET-Signed-off-by: Jiri Kosina <jkosina@...e.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
>
Powered by blists - more mailing lists