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 <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index b206115d761cb5bf5390386317e6b16ebe15b6d9..84708a5f8c520cb86db4ad62a4b845a5ec83b88f 100644 (file)
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3663,6 +3663,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) {
@@ -3706,21 +3711,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;