Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open

Denis Efremov
2021-01-21T14:48:05Z

On 1/21/21 1:25 PM, Jiri Kosina wrote:
> On Thu, 21 Jan 2021, Denis Efremov wrote:
>
>> I think it's hard to recall the exact reasons after so many years.
>
> Yeah, I guess so :)
>
>> I'll send a patch today based on this one.
>
> I am currently waiting for confirmation by the original reporter that the
> patch below fixes the issue.
>
>
>
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] floppy: reintroduce O_NDELAY fix
>
> Originally fixed in 09954bad4 ("floppy: refactor open() flags handling")
> then reverted for unknown reason in f2791e7eadf437 instead of taking
> the open(O_ACCMODE) for ioctl-only open fix, which had the changelog below
>
> ====
> Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
> side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
> this is being used setfdprm userspace for ioctl-only open().
>
> Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
> modes, while still keeping the original O_NDELAY bug fixed.
>
> Cc: stable@vger.kernel.org # v4.5+
> Reported-by: Wim Osterholt <wim@djo.tudelft.nl>
> Tested-by: Wim Osterholt <wim@djo.tudelft.nl>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> =====
>
> Fixes: 09954bad4 ("floppy: refactor open() flags handling")
> Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"")
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/block/floppy.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index dfe1dfc901cc..bda9417aa0a8 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4121,23 +4121,22 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> if (fdc_state[FDC(drive)].rawcmd == 1)
> fdc_state[FDC(drive)].rawcmd = 2;
>
> - if (!(mode & FMODE_NDELAY)) {
> - if (mode & (FMODE_READ|FMODE_WRITE)) {
> - drive_state[drive].last_checked = 0;
> - clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
> - &drive_state[drive].flags);
> - if (bdev_check_media_change(bdev))
> - floppy_revalidate(bdev->bd_disk);
> - if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags))
> - goto out;
> - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags))
> - goto out;
> - }
> - res = -EROFS;
> - if ((mode & FMODE_WRITE) &&
> - !test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags))
> + if (mode & (FMODE_READ|FMODE_WRITE)) {

As the bot points out this was refactored a bit in:
8d9d34e25a37 ("floppy: cleanup: expand macro UDRS")
4a6f3d480edc ("floppy: use bdev_check_media_change")

Should be something like:
+ drive_state[drive].last_checked = 0;
+ clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
+ &drive_state[drive].flags);
+ if (bdev_check_media_change(bdev))
+ floppy_revalidate(bdev->bd_disk);

> + if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags))
> + goto out;
> + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags))
> goto out;
> }
> +
> + res = -EROFS;
> +
> + if ((mode & FMODE_WRITE) &&
> + !test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags))
> + goto out;
> +
> mutex_unlock(&open_lock);
> mutex_unlock(&floppy_mutex);
> return 0;
>