| Message ID | nycvar.YFH.7.76.2101221209060.5622@cbobk.fhfr.pm (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | floppy: reintroduce O_NDELAY fix | expand |
On 1/22/21 2:13 PM, Jiri Kosina wrote: > From: Jiri Kosina <jkosina@suse.cz> > > This issue was originally fixed in 09954bad4 ("floppy: refactor open() > flags handling"). > > The fix as a side-effect, however, introduce issue for open(O_ACCMODE) > that is being used for ioctl-only open. I wrote a fix for that, but > instead of it being merged, full revert of 09954bad4 was performed, > re-introducing the O_NDELAY / O_NONBLOCK issue, and it strikes again. > > This is a forward-port of the original fix to current codebase; the > original submission 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 > Reported-by: Wim Osterholt <wim@djo.tudelft.nl> > Tested-by: Wim Osterholt <wim@djo.tudelft.nl> > Reported-and-tested-by: Kurt Garloff <kurt@garloff.de> > Fixes: 09954bad4 ("floppy: refactor open() flags handling") > Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") > Signed-off-by: Jiri Kosina <jkosina@suse.cz> Applied. I'll send it to Jens soon with a couple of cleanup patches. https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 Thanks, Denis > --- > drivers/block/floppy.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c > index dfe1dfc901cc..0b71292d9d5a 100644 > --- a/drivers/block/floppy.c > +++ b/drivers/block/floppy.c > @@ -4121,23 +4121,23 @@ 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)) { > + 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; >
Hi Denis, Jiri, Jens, Am 26.01.21 um 09:21 schrieb Denis Efremov: > On 1/22/21 2:13 PM, Jiri Kosina wrote: >> From: Jiri Kosina <jkosina@suse.cz> >> >> This issue was originally fixed in 09954bad4 ("floppy: refactor open() >> flags handling"). >> >> The fix as a side-effect, however, introduce issue for open(O_ACCMODE) >> that is being used for ioctl-only open. I wrote a fix for that, but >> instead of it being merged, full revert of 09954bad4 was performed, >> re-introducing the O_NDELAY / O_NONBLOCK issue, and it strikes again. >> >> This is a forward-port of the original fix to current codebase; the >> original submission 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 >> Reported-by: Wim Osterholt <wim@djo.tudelft.nl> >> Tested-by: Wim Osterholt <wim@djo.tudelft.nl> >> Reported-and-tested-by: Kurt Garloff <kurt@garloff.de> >> Fixes: 09954bad4 ("floppy: refactor open() flags handling") >> Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") >> Signed-off-by: Jiri Kosina <jkosina@suse.cz> > Applied. I'll send it to Jens soon with a couple of cleanup patches. > > https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 Great, thanks. Due to libblkid (rightfully) using O_NONBLOCK these days when probing devices, the floppy driver does spit loads of [ 9.533513] floppy0: disk absent or changed during operation [ 9.534989] blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0 [ 9.537206] Buffer I/O error on dev fd0, logical block 0, async page read [ 9.546837] floppy0: disk absent or changed during operation [ 9.548389] blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 and fails a mount prior to being opened without O_NONBLOCK at least once. (Reproduction is easy with qemu-kvm.) The patch addresses it and I would suggest it to also be backported and applied to the active stable kernel trees. Thanks,
On 1/26/21 12:31 PM, Kurt Garloff wrote: > Hi Denis, Jiri, Jens, > > Am 26.01.21 um 09:21 schrieb Denis Efremov: >> On 1/22/21 2:13 PM, Jiri Kosina wrote: >>> From: Jiri Kosina <jkosina@suse.cz> >>> >>> This issue was originally fixed in 09954bad4 ("floppy: refactor open() >>> flags handling"). >>> >>> The fix as a side-effect, however, introduce issue for open(O_ACCMODE) >>> that is being used for ioctl-only open. I wrote a fix for that, but >>> instead of it being merged, full revert of 09954bad4 was performed, >>> re-introducing the O_NDELAY / O_NONBLOCK issue, and it strikes again. >>> >>> This is a forward-port of the original fix to current codebase; the >>> original submission 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 >>> Reported-by: Wim Osterholt <wim@djo.tudelft.nl> >>> Tested-by: Wim Osterholt <wim@djo.tudelft.nl> >>> Reported-and-tested-by: Kurt Garloff <kurt@garloff.de> >>> Fixes: 09954bad4 ("floppy: refactor open() flags handling") >>> Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") >>> Signed-off-by: Jiri Kosina <jkosina@suse.cz> >> Applied. I'll send it to Jens soon with a couple of cleanup patches. >> >> https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 > > Great, thanks. > > Due to libblkid (rightfully) using O_NONBLOCK these days when probing > devices, the floppy driver does spit loads of > [ 9.533513] floppy0: disk absent or changed during operation > [ 9.534989] blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0 > [ 9.537206] Buffer I/O error on dev fd0, logical block 0, async page read > [ 9.546837] floppy0: disk absent or changed during operation > [ 9.548389] blk_update_request: I/O error, dev fd0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 > and fails a mount prior to being opened without O_NONBLOCK at least once. > (Reproduction is easy with qemu-kvm.) > > The patch addresses it and I would suggest it to also be backported and > applied to the active stable kernel trees. Yes, it will be backported to all stable trees including v4.4 Thanks, Denis
On Tue, 26 Jan 2021, Denis Efremov wrote: > Applied. I'll send it to Jens soon with a couple of cleanup patches. > > https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 Denis, I don't see this fix in Jens' tree yet. Is there any problem with the patch? Thanks,
On 2/4/21 12:24 PM, Jiri Kosina wrote: > On Tue, 26 Jan 2021, Denis Efremov wrote: > >> Applied. I'll send it to Jens soon with a couple of cleanup patches. >> >> https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 > > Denis, > > I don't see this fix in Jens' tree yet. Is there any problem with the > patch? Hi, sorry for the delay. I've just send the pull request to Jens. I tested the patch and stressed the driver with syzkaller. Everything look good with the patch to me. Thanks! Denis
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c index dfe1dfc901cc..0b71292d9d5a 100644 --- a/drivers/block/floppy.c +++ b/drivers/block/floppy.c @@ -4121,23 +4121,23 @@ 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)) { + 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;