Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On 1/21/21 5:44 PM, Jiri Kosina wrote: > On Thu, 21 Jan 2021, Jiri Kosina wrote: > >> I am currently waiting for confirmation by the original reporter that the >> patch below fixes the issue. > > ... a now a patch that actually compiles :) (made a mistake when > forward-porting from the older kernel on which this has been reported). Oh, sorry for the last message (forgot to check the inbox before hitting the send button). I'll test the patch. A couple of nitpicks below. > > From: Jiri Kosina > Subject: [PATCH v2] 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: sta...@vger.kernel.org # v4.5+ Are you sure that it's not worth to backport it to LTS v4.4? Because f2791e7ead is just a revert and 09954bad4 is not presented in v4.4 I'm not sure what fixes tag is better to use in this case. > Reported-by: Wim Osterholt > Tested-by: Wim Osterholt > Signed-off-by: Jiri Kosina > = > > Fixes: 09954bad4 ("floppy: refactor open() flags handling") > Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") > Signed-off-by: Jiri Kosina > --- > > v1 -> v2: fix build issue due to bad forward-port > > 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..f9e839c8c5aa 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, > - _state[drive].flags); > - if (bdev_check_media_change(bdev)) > - floppy_revalidate(bdev->bd_disk); > - if (test_bit(FD_DISK_CHANGED_BIT, > _state[drive].flags)) > - goto out; > - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, > _state[drive].flags)) > - goto out; > - } > - res = -EROFS; > - if ((mode & FMODE_WRITE) && > - !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags)) > + if (mode & (FMODE_READ|FMODE_WRITE)) { > + UDRS->last_checked = 0; UDRS will still break the compilation here. > + clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags); > + if (bdev_check_media_change(bdev)) > + floppy_revalidate(bdev->bd_disk); > + if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) > + goto out; > + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, > _state[drive].flags)) > goto out; > } > + > + res = -EROFS; > + > + if ((mode & FMODE_WRITE) && > + !test_bit(FD_DISK_WRITABLE_BIT, > _state[drive].flags)) > + goto out; > + > mutex_unlock(_lock); > mutex_unlock(_mutex); > return 0; >
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, 21 Jan 2021, Denis Efremov wrote: > > From: Jiri Kosina > > Subject: [PATCH v2] 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: sta...@vger.kernel.org # v4.5+ > > Are you sure that it's not worth to backport it to LTS v4.4? Because > f2791e7ead is just a revert and 09954bad4 is not presented in v4.4 I'm > not sure what fixes tag is better to use in this case. You are right; I'll drop the '4.5+' indicator and will backport it once/if it hits Linus' tree. > > + if (mode & (FMODE_READ|FMODE_WRITE)) { > > + UDRS->last_checked = 0; > > UDRS will still break the compilation here. Doh, forgot to refresh before sending, sorry for the noise. I'll send the final version once I get confirmation from the reporter that it's fixing the issue properly, add his Reported-by: etc. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
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 > 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: sta...@vger.kernel.org # v4.5+ > Reported-by: Wim Osterholt > Tested-by: Wim Osterholt > Signed-off-by: Jiri Kosina > = > > Fixes: 09954bad4 ("floppy: refactor open() flags handling") > Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") > Signed-off-by: Jiri Kosina > --- > 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, > - _state[drive].flags); > - if (bdev_check_media_change(bdev)) > - floppy_revalidate(bdev->bd_disk); > - if (test_bit(FD_DISK_CHANGED_BIT, > _state[drive].flags)) > - goto out; > - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, > _state[drive].flags)) > - goto out; > - } > - res = -EROFS; > - if ((mode & FMODE_WRITE) && > - !test_bit(FD_DISK_WRITABLE_BIT, _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, + _state[drive].flags); + if (bdev_check_media_change(bdev)) + floppy_revalidate(bdev->bd_disk); > + if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) > + goto out; > + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, > _state[drive].flags)) > goto out; > } > + > + res = -EROFS; > + > + if ((mode & FMODE_WRITE) && > + !test_bit(FD_DISK_WRITABLE_BIT, > _state[drive].flags)) > + goto out; > + > mutex_unlock(_lock); > mutex_unlock(_mutex); > return 0; >
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, 21 Jan 2021, Jiri Kosina wrote: > I am currently waiting for confirmation by the original reporter that the > patch below fixes the issue. ... a now a patch that actually compiles :) (made a mistake when forward-porting from the older kernel on which this has been reported). From: Jiri Kosina Subject: [PATCH v2] 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: sta...@vger.kernel.org # v4.5+ Reported-by: Wim Osterholt Tested-by: Wim Osterholt Signed-off-by: Jiri Kosina = Fixes: 09954bad4 ("floppy: refactor open() flags handling") Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") Signed-off-by: Jiri Kosina --- v1 -> v2: fix build issue due to bad forward-port 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..f9e839c8c5aa 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, - _state[drive].flags); - if (bdev_check_media_change(bdev)) - floppy_revalidate(bdev->bd_disk); - if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) - goto out; - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags)) - goto out; - } - res = -EROFS; - if ((mode & FMODE_WRITE) && - !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags)) + if (mode & (FMODE_READ|FMODE_WRITE)) { + UDRS->last_checked = 0; + clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags); + if (bdev_check_media_change(bdev)) + floppy_revalidate(bdev->bd_disk); + if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) + goto out; + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags)) goto out; } + + res = -EROFS; + + if ((mode & FMODE_WRITE) && + !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags)) + goto out; + mutex_unlock(_lock); mutex_unlock(_mutex); return 0; -- Jiri Kosina SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
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 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: sta...@vger.kernel.org # v4.5+ Reported-by: Wim Osterholt Tested-by: Wim Osterholt Signed-off-by: Jiri Kosina = Fixes: 09954bad4 ("floppy: refactor open() flags handling") Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"") Signed-off-by: Jiri Kosina --- 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, - _state[drive].flags); - if (bdev_check_media_change(bdev)) - floppy_revalidate(bdev->bd_disk); - if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) - goto out; - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags)) - goto out; - } - res = -EROFS; - if ((mode & FMODE_WRITE) && - !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags)) + if (mode & (FMODE_READ|FMODE_WRITE)) { + UDRS->last_checked = 0; + clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags); + check_disk_change(bdev); + if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags)) + goto out; + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags)) goto out; } + + res = -EROFS; + + if ((mode & FMODE_WRITE) && + !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags)) + goto out; + mutex_unlock(_lock); mutex_unlock(_mutex); return 0; -- Jiri Kosina SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
Hi, On 1/19/21 6:53 PM, Jiri Kosina wrote: > On Mon, 25 Jul 2016, Jens Axboe wrote: > >>> From: Jiri Kosina >>> >>> 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: sta...@vger.kernel.org # v4.5+ >>> Reported-by: Wim Osterholt >>> Tested-by: Wim Osterholt >>> Signed-off-by: Jiri Kosina >> >> Added for this series, thanks. > > [ CCing Denis too ] > > Let me revive this 4 years old thread. > > I've just now noticed that instead of my patch above being merged, what > happened instead was > > commit f2791e7eadf437633f30faa51b30878cf15650be > Author: Jens Axboe > Date: Thu Aug 25 08:56:51 2016 -0600 > > Revert "floppy: refactor open() flags handling" > > This reverts commit 09954bad448791ef01202351d437abdd9497a804. > > > which was plain revert of 09954bad4 (without any further explanation), > which in turn reintroduced the O_NDELAY issue, and I've just been hit by > it again. > > I am not able to find any e-mail thread that'd indicate why ultimately > revert happened, instead of mergin my fix. I think it's hard to recall the exact reasons after so many years. I'll send a patch today based on this one. Best Regards, Denis
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Mon, 25 Jul 2016, Jens Axboe wrote: > > From: Jiri Kosina > > > > 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: sta...@vger.kernel.org # v4.5+ > > Reported-by: Wim Osterholt > > Tested-by: Wim Osterholt > > Signed-off-by: Jiri Kosina > > Added for this series, thanks. [ CCing Denis too ] Let me revive this 4 years old thread. I've just now noticed that instead of my patch above being merged, what happened instead was commit f2791e7eadf437633f30faa51b30878cf15650be Author: Jens Axboe Date: Thu Aug 25 08:56:51 2016 -0600 Revert "floppy: refactor open() flags handling" This reverts commit 09954bad448791ef01202351d437abdd9497a804. which was plain revert of 09954bad4 (without any further explanation), which in turn reintroduced the O_NDELAY issue, and I've just been hit by it again. I am not able to find any e-mail thread that'd indicate why ultimately revert happened, instead of mergin my fix. Jens, do you have any idea? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On 06/30/2016 05:18 AM, Jiri Kosina wrote: From: Jiri KosinaCommit 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: sta...@vger.kernel.org # v4.5+ Reported-by: Wim Osterholt Tested-by: Wim Osterholt Signed-off-by: Jiri Kosina Added for this series, thanks. -- Jens Axboe
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On 06/30/2016 05:18 AM, Jiri Kosina wrote: From: Jiri Kosina 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: sta...@vger.kernel.org # v4.5+ Reported-by: Wim Osterholt Tested-by: Wim Osterholt Signed-off-by: Jiri Kosina Added for this series, thanks. -- Jens Axboe
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, Jun 30, 2016 at 01:18:00PM +0200, Jiri Kosina wrote: > From: Jiri Kosina> > 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: sta...@vger.kernel.org # v4.5+ > Reported-by: Wim Osterholt > Tested-by: Wim Osterholt > Signed-off-by: Jiri Kosina > --- > > Jens, this should preferably go into 4.7-rcX and to -stable as well. > How come this patch is still not in the recent release of 4.7 ? Regards, Wim.
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, Jun 30, 2016 at 01:18:00PM +0200, Jiri Kosina wrote: > From: Jiri Kosina > > 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: sta...@vger.kernel.org # v4.5+ > Reported-by: Wim Osterholt > Tested-by: Wim Osterholt > Signed-off-by: Jiri Kosina > --- > > Jens, this should preferably go into 4.7-rcX and to -stable as well. > How come this patch is still not in the recent release of 4.7 ? Regards, Wim.