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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 4 Feb 2016 22:22:04 +0100 (CET)
From:	Jiri Kosina <jikos@...nel.org>
To:	Dmitry Vyukov <dvyukov@...gle.com>
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: [PATCH] floppy: refactor open() flags handling (was Re: mm:
 uninterruptable tasks hanged on mmap_sem)

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.

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

Powered by Openwall GNU/*/Linux Powered by OpenVZ