All of lore.kernel.org
 help / color / mirror / Atom feed
* disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
@ 2016-06-10 23:02 Wim Osterholt
  2016-06-11 13:15 ` VFS regression ? " One Thousand Gnomes
  2016-06-13 12:15 ` Jiri Kosina
  0 siblings, 2 replies; 33+ messages in thread
From: Wim Osterholt @ 2016-06-10 23:02 UTC (permalink / raw)
  To: jikos; +Cc: Wim Osterholt, linux-kernel

L.S.

up to vanilla kernel 4.4.13 floppy functionality performs like it should.
(On an x86 PC that is. With a 1.44MB diskette drive.)
>From kernel 4.5* and up it changed to barely usable.

After a virgin start (cold or warm boot) with an empty diskette drive and
then loaded with a standard 720K diskette you may run 'mdir' (from mtools)
and it shows the directory fine.
The first time you load a standard 1.44MB diskette (wether it is a virgin
start or after a 720K disktette) and you run mdir, it says literally:

 plain_io: Input/output error
 init A: could not read boot sector
 Cannot initialize 'A:'

After this, all subsequent runs of mdir will do fine on both floppies.
However, most of my floppies are in a different format. (1.6MB)
I rely on 'setfdprm' (from fdutils) to set the correct parameters.

setfdprm /dev/fd0 1600/1440
/dev/fd0: Invalid argument

Strace shows me:
...
open(/dev/fd0, O_ACCMODE) = -1

So this actually means that 'invalid argument' refers to O_ACCMODE.

At all kernels I see no changed rights in:
ls -al /dev/fd0
 brw-rw---- 1 root disk 2, 0 May 13 16:43 /dev/fd0

It looks to me that the code in floppy.c is quite old; no changes here.
So the bug is elsewhere in the kernel.

Could someone please explain and repair the magic that is happening here?

Thanks in advance, Wim Osterholt.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* VFS regression ? Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-10 23:02 disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
@ 2016-06-11 13:15 ` One Thousand Gnomes
  2016-06-12  0:23   ` Wim Osterholt
  2016-06-13 12:15 ` Jiri Kosina
  1 sibling, 1 reply; 33+ messages in thread
From: One Thousand Gnomes @ 2016-06-11 13:15 UTC (permalink / raw)
  To: Wim Osterholt; +Cc: jikos, linux-kernel, Al Viro

On Sat, 11 Jun 2016 01:02:55 +0200
Wim Osterholt <wim@djo.tudelft.nl> wrote:

> L.S.
> 
> up to vanilla kernel 4.4.13 floppy functionality performs like it should.
> (On an x86 PC that is. With a 1.44MB diskette drive.)
> >From kernel 4.5* and up it changed to barely usable.  
> 
> After a virgin start (cold or warm boot) with an empty diskette drive and
> then loaded with a standard 720K diskette you may run 'mdir' (from mtools)
> and it shows the directory fine.
> The first time you load a standard 1.44MB diskette (wether it is a virgin
> start or after a 720K disktette) and you run mdir, it says literally:
> 
>  plain_io: Input/output error
>  init A: could not read boot sector
>  Cannot initialize 'A:'
> 
> After this, all subsequent runs of mdir will do fine on both floppies.
> However, most of my floppies are in a different format. (1.6MB)
> I rely on 'setfdprm' (from fdutils) to set the correct parameters.
> 
> setfdprm /dev/fd0 1600/1440
> /dev/fd0: Invalid argument
> 
> Strace shows me:
> ...
> open(/dev/fd0, O_ACCMODE) = -1
> 
> So this actually means that 'invalid argument' refers to O_ACCMODE.

That looks like a bug introduced in the VFS layer rather than in the
floppy driver. A couple of Linux drivers have a weird un-Unixy hack where
you open them with a mode value of 3 to mean "control only" and /dev/fd
is the main user of this

If you do 

touch foo

then compile and run the following program does it error on the newer
kernel ?

#include <fcntl.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
  if (open("foo", 3) == -1)
    perror("foo");
  return 0;
}



Alan

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: VFS regression ? Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-11 13:15 ` VFS regression ? " One Thousand Gnomes
@ 2016-06-12  0:23   ` Wim Osterholt
  0 siblings, 0 replies; 33+ messages in thread
From: Wim Osterholt @ 2016-06-12  0:23 UTC (permalink / raw)
  To: One Thousand Gnomes; +Cc: jikos, linux-kernel, Al Viro, Wim Osterholt

On Sat, Jun 11, 2016 at 02:15:01PM +0100, One Thousand Gnomes wrote:

> > open(/dev/fd0, O_ACCMODE) = -1

> If you do 
> 
> touch foo
> 
> then compile and run the following program does it error on the newer
> kernel ?
> 
> #include <fcntl.h>
> #include <stdio.h>
> 
> int main(int argc, char *argv[])
> {
>   if (open("foo", 3) == -1)
>     perror("foo");
>   return 0;
> }

No errors get printed for kernel 4.4 to 4.7 .

Regards, Wim.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-10 23:02 disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
  2016-06-11 13:15 ` VFS regression ? " One Thousand Gnomes
@ 2016-06-13 12:15 ` Jiri Kosina
  2016-06-14 18:43   ` Wim Osterholt
  2016-06-14 19:09   ` Al Viro
  1 sibling, 2 replies; 33+ messages in thread
From: Jiri Kosina @ 2016-06-13 12:15 UTC (permalink / raw)
  To: Wim Osterholt; +Cc: linux-kernel

On Sat, 11 Jun 2016, Wim Osterholt wrote:

> 
> up to vanilla kernel 4.4.13 floppy functionality performs like it should.
> (On an x86 PC that is. With a 1.44MB diskette drive.)
> >From kernel 4.5* and up it changed to barely usable.
> 
> After a virgin start (cold or warm boot) with an empty diskette drive and
> then loaded with a standard 720K diskette you may run 'mdir' (from mtools)
> and it shows the directory fine.
> The first time you load a standard 1.44MB diskette (wether it is a virgin
> start or after a 720K disktette) and you run mdir, it says literally:
> 
>  plain_io: Input/output error
>  init A: could not read boot sector
>  Cannot initialize 'A:'
> 
> After this, all subsequent runs of mdir will do fine on both floppies.
> However, most of my floppies are in a different format. (1.6MB)
> I rely on 'setfdprm' (from fdutils) to set the correct parameters.
> 
> setfdprm /dev/fd0 1600/1440
> /dev/fd0: Invalid argument
> 
> Strace shows me:
> ...
> open(/dev/fd0, O_ACCMODE) = -1
> 
> So this actually means that 'invalid argument' refers to O_ACCMODE.

Hmm, could you please test with 09954bad448 reverted? (although I don't 
really have a good explanation currently how it'd be causing what you are 
observing).

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-13 12:15 ` Jiri Kosina
@ 2016-06-14 18:43   ` Wim Osterholt
  2016-06-15  7:09     ` Jiri Kosina
  2016-06-14 19:09   ` Al Viro
  1 sibling, 1 reply; 33+ messages in thread
From: Wim Osterholt @ 2016-06-14 18:43 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: viro, Wim Osterholt, linux-kernel, gnomes


On Mon, Jun 13, 2016 at 02:15:15PM +0200, Jiri Kosina wrote:
> > up to vanilla kernel 4.4.13 floppy functionality performs like it should.
> > (On an x86 PC that is. With a 1.44MB diskette drive.)
> > >From kernel 4.5* and up it changed to barely usable.
> > 
> > After a virgin start (cold or warm boot) with an empty diskette drive and
> > then loaded with a standard 720K diskette you may run 'mdir' (from mtools)
> > and it shows the directory fine.
> > The first time you load a standard 1.44MB diskette (wether it is a virgin
> > start or after a 720K disktette) and you run mdir, it says literally:
> > 
> >  plain_io: Input/output error
> >  init A: could not read boot sector
> >  Cannot initialize 'A:'
> > 
> > After this, all subsequent runs of mdir will do fine on both floppies.
> > However, most of my floppies are in a different format. (1.6MB)
> > I rely on 'setfdprm' (from fdutils) to set the correct parameters.
> > 
> > setfdprm /dev/fd0 1600/1440
> > /dev/fd0: Invalid argument
> > 
> > Strace shows me:
> > ...
> > open(/dev/fd0, O_ACCMODE) = -1
> > 
> > So this actually means that 'invalid argument' refers to O_ACCMODE.
> 
> Hmm, could you please test with 09954bad448 reverted? (although I don't 
> really have a good explanation currently how it'd be causing what you are 
> observing).
> 
> Thanks,
> 
> -- 
> Jiri Kosina

Hmm. Now I need a crash course git.
After reading at www.kernel.org/pub/software/scm/git/docs/user-manual.html
I now tried:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git checkout -b new v4.5  (assuming that the first 'wrong' kernel would be best)
git revert 09954bad448    (that did something, which I assume te be good)
copied the .config file from 4.5 I had lying around  and ran make.

Surprising or not, the thusly compiled kernel ran fine and I could handle
floppies like before!
(open(/dev/fd0,O_ACCMODE) succeeds.)


Regards, Wim.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-13 12:15 ` Jiri Kosina
  2016-06-14 18:43   ` Wim Osterholt
@ 2016-06-14 19:09   ` Al Viro
  1 sibling, 0 replies; 33+ messages in thread
From: Al Viro @ 2016-06-14 19:09 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Wim Osterholt, linux-kernel

On Mon, Jun 13, 2016 at 02:15:15PM +0200, Jiri Kosina wrote:

> Hmm, could you please test with 09954bad448 reverted? (although I don't 
> really have a good explanation currently how it'd be causing what you are 
> observing).

I do, actually - ->f_mode on open(..., 3) contains neither FMODE_READ nor
FMODE_WRITE.  So this
    While at it, clean up a bit handling of !(mode & (FMODE_READ|FMODE_WRITE))
    case and return EINVAL instead of succeeding as well.
is working as promised in commit message.

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-14 18:43   ` Wim Osterholt
@ 2016-06-15  7:09     ` Jiri Kosina
  2016-06-15 11:42       ` Wim Osterholt
  2016-06-15 13:20       ` Al Viro
  0 siblings, 2 replies; 33+ messages in thread
From: Jiri Kosina @ 2016-06-15  7:09 UTC (permalink / raw)
  To: Wim Osterholt; +Cc: viro, linux-kernel, gnomes

On Tue, 14 Jun 2016, Wim Osterholt wrote:

> Surprising or not, the thusly compiled kernel ran fine and I could 
> handle floppies like before! (open(/dev/fd0,O_ACCMODE) succeeds.)

Thanks for testing.

Now next question -- what do you actually want to achieve with passing 
O_ACCMODE to open()?

O_ACCMODE should primarily be used as a mask to use when extracting access 
mode bits from fcntl(F_GETFL) call.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15  7:09     ` Jiri Kosina
@ 2016-06-15 11:42       ` Wim Osterholt
  2016-06-15 13:20       ` Al Viro
  1 sibling, 0 replies; 33+ messages in thread
From: Wim Osterholt @ 2016-06-15 11:42 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: viro, linux-kernel, gnomes, Wim Osterholt

On Wed, Jun 15, 2016 at 09:09:13AM +0200, Jiri Kosina wrote:
> > Surprising or not, the thusly compiled kernel ran fine and I could 
> > handle floppies like before! (open(/dev/fd0,O_ACCMODE) succeeds.)
> 
> Thanks for testing.
> 
> Now next question -- what do you actually want to achieve with passing 
> O_ACCMODE to open()?
> 
> O_ACCMODE should primarily be used as a mask to use when extracting access 
> mode bits from fcntl(F_GETFL) call.

It happens in setfdprm from fdutils. What they wanted to achieve with it
I don't know. Setting parameters or some such.
Problem is that fdutils is probably unmaintained for ten years or so.

Regards, Wim.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15  7:09     ` Jiri Kosina
  2016-06-15 11:42       ` Wim Osterholt
@ 2016-06-15 13:20       ` Al Viro
  2016-06-15 14:13         ` Jiri Kosina
  1 sibling, 1 reply; 33+ messages in thread
From: Al Viro @ 2016-06-15 13:20 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Wim Osterholt, linux-kernel, gnomes

On Wed, Jun 15, 2016 at 09:09:13AM +0200, Jiri Kosina wrote:
> On Tue, 14 Jun 2016, Wim Osterholt wrote:
> 
> > Surprising or not, the thusly compiled kernel ran fine and I could 
> > handle floppies like before! (open(/dev/fd0,O_ACCMODE) succeeds.)
> 
> Thanks for testing.
> 
> Now next question -- what do you actually want to achieve with passing 
> O_ACCMODE to open()?
> 
> O_ACCMODE should primarily be used as a mask to use when extracting access 
> mode bits from fcntl(F_GETFL) call.

ioctl-only open.  It's an old weird part of /dev/fd0 ABI and if you are
playing with that driver, you'd better bother to check the actual userland
talking to it.

Rationale, IIRC, is that unlike the normal open() this one does *not* depend
on formatted disk being there.  Regularizing it ot of existence is not
a good idea.

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15 13:20       ` Al Viro
@ 2016-06-15 14:13         ` Jiri Kosina
  2016-06-15 22:47           ` Wim Osterholt
  2016-06-15 23:07           ` disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
  0 siblings, 2 replies; 33+ messages in thread
From: Jiri Kosina @ 2016-06-15 14:13 UTC (permalink / raw)
  To: Al Viro; +Cc: Wim Osterholt, linux-kernel, gnomes

On Wed, 15 Jun 2016, Al Viro wrote:

> ioctl-only open.  It's an old weird part of /dev/fd0 ABI 

Ah, right you are, I completely forgot about this gem.

> and if you are playing with that driver, 

I am merely trying to keep it in a state that doesn't crash the system.

> you'd better bother to check the actual userland talking to it.

Sure, this needs to be fixed.

Wim, could you please test whether the patch below, applied on top of 
vanilla kernel (i.e. drop the revert), everything you are using still 
works as expected?




From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] floppy: fix open(O_ACCMODE) for ioctl-only open

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.

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 drivers/block/floppy.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 84708a5..a1dcf12 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3663,11 +3663,6 @@ 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) {
@@ -3711,13 +3706,15 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 	if (UFDCS->rawcmd == 1)
 		UFDCS->rawcmd = 2;
 
-	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;
+	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;
 
-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply related	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15 14:13         ` Jiri Kosina
@ 2016-06-15 22:47           ` Wim Osterholt
  2016-06-16  7:53             ` [PATCH] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
  2016-06-15 23:07           ` disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
  1 sibling, 1 reply; 33+ messages in thread
From: Wim Osterholt @ 2016-06-15 22:47 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Al Viro, linux-kernel, gnomes, Wim Osterholt

On Wed, Jun 15, 2016 at 04:13:53PM +0200, Jiri Kosina wrote:
> 
> Wim, could you please test whether the patch below, applied on top of 
> vanilla kernel (i.e. drop the revert), everything you are using still 
> works as expected?
> 

Applied on kernel-4.7-rc3 it looks like it's working. (Strace setfdprm looks
good.) That is on a remote machine. An actual test on floppies must wait
until tomorrow when I get there.

Regards, Wim.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15 14:13         ` Jiri Kosina
  2016-06-15 22:47           ` Wim Osterholt
@ 2016-06-15 23:07           ` Wim Osterholt
  2016-06-15 23:12             ` Jiri Kosina
  2016-06-15 23:13             ` Joe Perches
  1 sibling, 2 replies; 33+ messages in thread
From: Wim Osterholt @ 2016-06-15 23:07 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Al Viro, linux-kernel, gnomes, Wim Osterholt

On my first message I stated:

 It looks to me that the code in floppy.c is quite old; no changes here.
 So the bug is elsewhere in the kernel.
 
That was because the changelog at the beginning of floppy.c ended in 2003.
Wouln't it be wise to keep these items updated?

Groeten, Wim.


----- wim@djo.tudelft.nl -----

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15 23:07           ` disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
@ 2016-06-15 23:12             ` Jiri Kosina
  2016-06-15 23:13             ` Joe Perches
  1 sibling, 0 replies; 33+ messages in thread
From: Jiri Kosina @ 2016-06-15 23:12 UTC (permalink / raw)
  To: Wim Osterholt; +Cc: Al Viro, linux-kernel, gnomes

On Thu, 16 Jun 2016, Wim Osterholt wrote:

> That was because the changelog at the beginning of floppy.c ended in 2003.
> Wouln't it be wise to keep these items updated?

Those things have only historical value these days. The real changelog has 
been kept in git (formerly bitkeeper) changelogs for past ~15 years.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: disfunctional floppy driver in kernels 4.5, 4.6 and 4.7
  2016-06-15 23:07           ` disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
  2016-06-15 23:12             ` Jiri Kosina
@ 2016-06-15 23:13             ` Joe Perches
  1 sibling, 0 replies; 33+ messages in thread
From: Joe Perches @ 2016-06-15 23:13 UTC (permalink / raw)
  To: wim, Jiri Kosina; +Cc: Al Viro, linux-kernel, gnomes

On Thu, 2016-06-16 at 01:07 +0200, Wim Osterholt wrote:
> On my first message I stated:
> 
>  It looks to me that the code in floppy.c is quite old; no changes here.
>  So the bug is elsewhere in the kernel.
>  
> That was because the changelog at the beginning of floppy.c ended in 2003.
> Wouln't it be wise to keep these items updated?

Not really.

git commit log entries are a better place because they
can be relatively free-form, detailed and verbose.

^ permalink raw reply	[flat|nested] 33+ messages in thread
* [PATCH] floppy: fix open(O_ACCMODE) for ioctl-only open
  2016-06-15 22:47           ` Wim Osterholt
@ 2016-06-16  7:53             ` Jiri Kosina
  2016-06-30 11:18               ` [PATCH RESEND] " Jiri Kosina
  0 siblings, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2016-06-16  7:53 UTC (permalink / raw)
  To: Wim Osterholt, Jens Axboe; +Cc: linux-kernel, linux-block

From: Jiri Kosina <jkosina@suse.cz>

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>
---

Jens, this should preferably go into 4.7-rcX and to -stable as well.

 drivers/block/floppy.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 84708a5..a1dcf12 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3663,11 +3663,6 @@ 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) {
@@ -3711,13 +3706,15 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 	if (UFDCS->rawcmd == 1)
 		UFDCS->rawcmd = 2;
 
-	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;
+	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;
 
-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply related	[flat|nested] 33+ messages in thread
* [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2016-06-16  7:53             ` [PATCH] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
@ 2016-06-30 11:18               ` Jiri Kosina
  2016-07-25 18:09                 ` Wim Osterholt
  2016-07-25 20:48                 ` Jens Axboe
  0 siblings, 2 replies; 33+ messages in thread
From: Jiri Kosina @ 2016-06-30 11:18 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, linux-block, Wim Osterholt

From: Jiri Kosina <jkosina@suse.cz>

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>
---

Jens, this should preferably go into 4.7-rcX and to -stable as well.

 drivers/block/floppy.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 84708a5..a1dcf12 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3663,11 +3663,6 @@ 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) {
@@ -3711,13 +3706,15 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
 	if (UFDCS->rawcmd == 1)
 		UFDCS->rawcmd = 2;
 
-	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;
+	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;
 
-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply related	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2016-06-30 11:18               ` [PATCH RESEND] " Jiri Kosina
@ 2016-07-25 18:09                 ` Wim Osterholt
  2016-07-25 20:48                 ` Jens Axboe
  1 sibling, 0 replies; 33+ messages in thread
From: Wim Osterholt @ 2016-07-25 18:09 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt

On Thu, Jun 30, 2016 at 01:18:00PM +0200, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> 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>
> ---
> 
> 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.

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2016-06-30 11:18               ` [PATCH RESEND] " Jiri Kosina
  2016-07-25 18:09                 ` Wim Osterholt
@ 2016-07-25 20:48                 ` Jens Axboe
  2021-01-19 15:53                   ` Jiri Kosina
  1 sibling, 1 reply; 33+ messages in thread
From: Jens Axboe @ 2016-07-25 20:48 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-kernel, linux-block, Wim Osterholt

On 06/30/2016 05:18 AM, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
>
> 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>

Added for this series, thanks.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2016-07-25 20:48                 ` Jens Axboe
@ 2021-01-19 15:53                   ` Jiri Kosina
  2021-01-21  4:44                     ` Denis Efremov
  0 siblings, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2021-01-19 15:53 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, linux-block, Wim Osterholt, Denis Efremov

On Mon, 25 Jul 2016, Jens Axboe wrote:

> > From: Jiri Kosina <jkosina@suse.cz>
> >
> > 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>
> 
> 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 <axboe@fb.com>
	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


^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-19 15:53                   ` Jiri Kosina
@ 2021-01-21  4:44                     ` Denis Efremov
  2021-01-21 10:25                       ` Jiri Kosina
  0 siblings, 1 reply; 33+ messages in thread
From: Denis Efremov @ 2021-01-21  4:44 UTC (permalink / raw)
  To: Jiri Kosina, Jens Axboe; +Cc: linux-kernel, linux-block, Wim Osterholt

Hi,

On 1/19/21 6:53 PM, Jiri Kosina wrote:
> On Mon, 25 Jul 2016, Jens Axboe wrote:
> 
>>> From: Jiri Kosina <jkosina@suse.cz>
>>>
>>> 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>
>>
>> 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 <axboe@fb.com>
> 	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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-21  4:44                     ` Denis Efremov
@ 2021-01-21 10:25                       ` Jiri Kosina
  2021-01-21 13:28                           ` kernel test robot
                                           ` (2 more replies)
  0 siblings, 3 replies; 33+ messages in thread
From: Jiri Kosina @ 2021-01-21 10:25 UTC (permalink / raw)
  To: Denis Efremov; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt

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)) {
+		UDRS->last_checked = 0;
+		clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags);
+		check_disk_change(bdev);
+		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;

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply related	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-21 10:25                       ` Jiri Kosina
@ 2021-01-21 13:28                           ` kernel test robot
  2021-01-21 14:44                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
  2021-01-21 14:45                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Denis Efremov
  2 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2021-01-21 13:28 UTC (permalink / raw)
  To: Jiri Kosina, Denis Efremov
  Cc: kbuild-all, Jens Axboe, linux-kernel, linux-block, Wim Osterholt

[-- Attachment #1: Type: text/plain, Size: 6802 bytes --]

Hi Jiri,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on linux/master linus/master v5.11-rc4 next-20210121]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/floppy-reintroduce-O_NDELAY-fix/20210121-182951
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: sparc64-randconfig-r033-20210121 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/605da67173ab7c362845b2f74c2914bfcec6db2e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jiri-Kosina/floppy-reintroduce-O_NDELAY-fix/20210121-182951
        git checkout 605da67173ab7c362845b2f74c2914bfcec6db2e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/sparc/include/asm/floppy.h:5,
                    from drivers/block/floppy.c:251:
   arch/sparc/include/asm/floppy_64.h:200:13: warning: no previous prototype for 'sparc_floppy_irq' [-Wmissing-prototypes]
     200 | irqreturn_t sparc_floppy_irq(int irq, void *dev_cookie)
         |             ^~~~~~~~~~~~~~~~
   In file included from arch/sparc/include/asm/floppy.h:5,
                    from drivers/block/floppy.c:251:
   arch/sparc/include/asm/floppy_64.h:437:6: warning: no previous prototype for 'sun_pci_fd_dma_callback' [-Wmissing-prototypes]
     437 | void sun_pci_fd_dma_callback(struct ebus_dma_info *p, int event, void *cookie)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/block/floppy.c: In function 'floppy_open':
>> drivers/block/floppy.c:4125:3: error: 'UDRS' undeclared (first use in this function)
    4125 |   UDRS->last_checked = 0;
         |   ^~~~
   drivers/block/floppy.c:4125:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/block/floppy.c:4127:3: error: implicit declaration of function 'check_disk_change'; did you mean 'bdev_disk_changed'? [-Werror=implicit-function-declaration]
    4127 |   check_disk_change(bdev);
         |   ^~~~~~~~~~~~~~~~~
         |   bdev_disk_changed
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for COMPAT_BINFMT_ELF
   Depends on COMPAT && BINFMT_ELF
   Selected by
   - COMPAT && SPARC64
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS || MCOUNT
   Selected by
   - LOCKDEP && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86


vim +/UDRS +4125 drivers/block/floppy.c

  4052	
  4053	/*
  4054	 * floppy_open check for aliasing (/dev/fd0 can be the same as
  4055	 * /dev/PS0 etc), and disallows simultaneous access to the same
  4056	 * drive with different device numbers.
  4057	 */
  4058	static int floppy_open(struct block_device *bdev, fmode_t mode)
  4059	{
  4060		int drive = (long)bdev->bd_disk->private_data;
  4061		int old_dev, new_dev;
  4062		int try;
  4063		int res = -EBUSY;
  4064		char *tmp;
  4065	
  4066		mutex_lock(&floppy_mutex);
  4067		mutex_lock(&open_lock);
  4068		old_dev = drive_state[drive].fd_device;
  4069		if (opened_bdev[drive] && opened_bdev[drive] != bdev)
  4070			goto out2;
  4071	
  4072		if (!drive_state[drive].fd_ref && (drive_params[drive].flags & FD_BROKEN_DCL)) {
  4073			set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
  4074			set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
  4075		}
  4076	
  4077		drive_state[drive].fd_ref++;
  4078	
  4079		opened_bdev[drive] = bdev;
  4080	
  4081		res = -ENXIO;
  4082	
  4083		if (!floppy_track_buffer) {
  4084			/* if opening an ED drive, reserve a big buffer,
  4085			 * else reserve a small one */
  4086			if ((drive_params[drive].cmos == 6) || (drive_params[drive].cmos == 5))
  4087				try = 64;	/* Only 48 actually useful */
  4088			else
  4089				try = 32;	/* Only 24 actually useful */
  4090	
  4091			tmp = (char *)fd_dma_mem_alloc(1024 * try);
  4092			if (!tmp && !floppy_track_buffer) {
  4093				try >>= 1;	/* buffer only one side */
  4094				INFBOUND(try, 16);
  4095				tmp = (char *)fd_dma_mem_alloc(1024 * try);
  4096			}
  4097			if (!tmp && !floppy_track_buffer)
  4098				fallback_on_nodma_alloc(&tmp, 2048 * try);
  4099			if (!tmp && !floppy_track_buffer) {
  4100				DPRINT("Unable to allocate DMA memory\n");
  4101				goto out;
  4102			}
  4103			if (floppy_track_buffer) {
  4104				if (tmp)
  4105					fd_dma_mem_free((unsigned long)tmp, try * 1024);
  4106			} else {
  4107				buffer_min = buffer_max = -1;
  4108				floppy_track_buffer = tmp;
  4109				max_buffer_sectors = try;
  4110			}
  4111		}
  4112	
  4113		new_dev = MINOR(bdev->bd_dev);
  4114		drive_state[drive].fd_device = new_dev;
  4115		set_capacity(disks[drive][ITYPE(new_dev)], floppy_sizes[new_dev]);
  4116		if (old_dev != -1 && old_dev != new_dev) {
  4117			if (buffer_drive == drive)
  4118				buffer_track = -1;
  4119		}
  4120	
  4121		if (fdc_state[FDC(drive)].rawcmd == 1)
  4122			fdc_state[FDC(drive)].rawcmd = 2;
  4123	
  4124		if (mode & (FMODE_READ|FMODE_WRITE)) {
> 4125			UDRS->last_checked = 0;
  4126			clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags);
> 4127			check_disk_change(bdev);
  4128			if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags))
  4129				goto out;
  4130			if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags))
  4131				goto out;
  4132		}
  4133	
  4134		res = -EROFS;
  4135	
  4136		if ((mode & FMODE_WRITE) &&
  4137				!test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags))
  4138			goto out;
  4139	
  4140		mutex_unlock(&open_lock);
  4141		mutex_unlock(&floppy_mutex);
  4142		return 0;
  4143	out:
  4144		drive_state[drive].fd_ref--;
  4145	
  4146		if (!drive_state[drive].fd_ref)
  4147			opened_bdev[drive] = NULL;
  4148	out2:
  4149		mutex_unlock(&open_lock);
  4150		mutex_unlock(&floppy_mutex);
  4151		return res;
  4152	}
  4153	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25754 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
@ 2021-01-21 13:28                           ` kernel test robot
  0 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2021-01-21 13:28 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6973 bytes --]

Hi Jiri,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on linux/master linus/master v5.11-rc4 next-20210121]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/floppy-reintroduce-O_NDELAY-fix/20210121-182951
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: sparc64-randconfig-r033-20210121 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/605da67173ab7c362845b2f74c2914bfcec6db2e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jiri-Kosina/floppy-reintroduce-O_NDELAY-fix/20210121-182951
        git checkout 605da67173ab7c362845b2f74c2914bfcec6db2e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/sparc/include/asm/floppy.h:5,
                    from drivers/block/floppy.c:251:
   arch/sparc/include/asm/floppy_64.h:200:13: warning: no previous prototype for 'sparc_floppy_irq' [-Wmissing-prototypes]
     200 | irqreturn_t sparc_floppy_irq(int irq, void *dev_cookie)
         |             ^~~~~~~~~~~~~~~~
   In file included from arch/sparc/include/asm/floppy.h:5,
                    from drivers/block/floppy.c:251:
   arch/sparc/include/asm/floppy_64.h:437:6: warning: no previous prototype for 'sun_pci_fd_dma_callback' [-Wmissing-prototypes]
     437 | void sun_pci_fd_dma_callback(struct ebus_dma_info *p, int event, void *cookie)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/block/floppy.c: In function 'floppy_open':
>> drivers/block/floppy.c:4125:3: error: 'UDRS' undeclared (first use in this function)
    4125 |   UDRS->last_checked = 0;
         |   ^~~~
   drivers/block/floppy.c:4125:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/block/floppy.c:4127:3: error: implicit declaration of function 'check_disk_change'; did you mean 'bdev_disk_changed'? [-Werror=implicit-function-declaration]
    4127 |   check_disk_change(bdev);
         |   ^~~~~~~~~~~~~~~~~
         |   bdev_disk_changed
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for COMPAT_BINFMT_ELF
   Depends on COMPAT && BINFMT_ELF
   Selected by
   - COMPAT && SPARC64
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS || MCOUNT
   Selected by
   - LOCKDEP && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86


vim +/UDRS +4125 drivers/block/floppy.c

  4052	
  4053	/*
  4054	 * floppy_open check for aliasing (/dev/fd0 can be the same as
  4055	 * /dev/PS0 etc), and disallows simultaneous access to the same
  4056	 * drive with different device numbers.
  4057	 */
  4058	static int floppy_open(struct block_device *bdev, fmode_t mode)
  4059	{
  4060		int drive = (long)bdev->bd_disk->private_data;
  4061		int old_dev, new_dev;
  4062		int try;
  4063		int res = -EBUSY;
  4064		char *tmp;
  4065	
  4066		mutex_lock(&floppy_mutex);
  4067		mutex_lock(&open_lock);
  4068		old_dev = drive_state[drive].fd_device;
  4069		if (opened_bdev[drive] && opened_bdev[drive] != bdev)
  4070			goto out2;
  4071	
  4072		if (!drive_state[drive].fd_ref && (drive_params[drive].flags & FD_BROKEN_DCL)) {
  4073			set_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags);
  4074			set_bit(FD_VERIFY_BIT, &drive_state[drive].flags);
  4075		}
  4076	
  4077		drive_state[drive].fd_ref++;
  4078	
  4079		opened_bdev[drive] = bdev;
  4080	
  4081		res = -ENXIO;
  4082	
  4083		if (!floppy_track_buffer) {
  4084			/* if opening an ED drive, reserve a big buffer,
  4085			 * else reserve a small one */
  4086			if ((drive_params[drive].cmos == 6) || (drive_params[drive].cmos == 5))
  4087				try = 64;	/* Only 48 actually useful */
  4088			else
  4089				try = 32;	/* Only 24 actually useful */
  4090	
  4091			tmp = (char *)fd_dma_mem_alloc(1024 * try);
  4092			if (!tmp && !floppy_track_buffer) {
  4093				try >>= 1;	/* buffer only one side */
  4094				INFBOUND(try, 16);
  4095				tmp = (char *)fd_dma_mem_alloc(1024 * try);
  4096			}
  4097			if (!tmp && !floppy_track_buffer)
  4098				fallback_on_nodma_alloc(&tmp, 2048 * try);
  4099			if (!tmp && !floppy_track_buffer) {
  4100				DPRINT("Unable to allocate DMA memory\n");
  4101				goto out;
  4102			}
  4103			if (floppy_track_buffer) {
  4104				if (tmp)
  4105					fd_dma_mem_free((unsigned long)tmp, try * 1024);
  4106			} else {
  4107				buffer_min = buffer_max = -1;
  4108				floppy_track_buffer = tmp;
  4109				max_buffer_sectors = try;
  4110			}
  4111		}
  4112	
  4113		new_dev = MINOR(bdev->bd_dev);
  4114		drive_state[drive].fd_device = new_dev;
  4115		set_capacity(disks[drive][ITYPE(new_dev)], floppy_sizes[new_dev]);
  4116		if (old_dev != -1 && old_dev != new_dev) {
  4117			if (buffer_drive == drive)
  4118				buffer_track = -1;
  4119		}
  4120	
  4121		if (fdc_state[FDC(drive)].rawcmd == 1)
  4122			fdc_state[FDC(drive)].rawcmd = 2;
  4123	
  4124		if (mode & (FMODE_READ|FMODE_WRITE)) {
> 4125			UDRS->last_checked = 0;
  4126			clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags);
> 4127			check_disk_change(bdev);
  4128			if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags))
  4129				goto out;
  4130			if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags))
  4131				goto out;
  4132		}
  4133	
  4134		res = -EROFS;
  4135	
  4136		if ((mode & FMODE_WRITE) &&
  4137				!test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags))
  4138			goto out;
  4139	
  4140		mutex_unlock(&open_lock);
  4141		mutex_unlock(&floppy_mutex);
  4142		return 0;
  4143	out:
  4144		drive_state[drive].fd_ref--;
  4145	
  4146		if (!drive_state[drive].fd_ref)
  4147			opened_bdev[drive] = NULL;
  4148	out2:
  4149		mutex_unlock(&open_lock);
  4150		mutex_unlock(&floppy_mutex);
  4151		return res;
  4152	}
  4153	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 25754 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-21 10:25                       ` Jiri Kosina
  2021-01-21 13:28                           ` kernel test robot
@ 2021-01-21 14:44                         ` Jiri Kosina
  2021-01-21 15:02                           ` Denis Efremov
  2021-01-21 14:45                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Denis Efremov
  2 siblings, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2021-01-21 14:44 UTC (permalink / raw)
  To: Denis Efremov; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt

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 <jkosina@suse.cz>
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: 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>
---

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,
-				  &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)) {
+		UDRS->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;

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply related	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-21 10:25                       ` Jiri Kosina
  2021-01-21 13:28                           ` kernel test robot
  2021-01-21 14:44                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
@ 2021-01-21 14:45                         ` Denis Efremov
  2 siblings, 0 replies; 33+ messages in thread
From: Denis Efremov @ 2021-01-21 14:45 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt

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;
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-21 14:44                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
@ 2021-01-21 15:02                           ` Denis Efremov
  2021-01-21 15:05                             ` Jiri Kosina
  0 siblings, 1 reply; 33+ messages in thread
From: Denis Efremov @ 2021-01-21 15:02 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt



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 <jkosina@suse.cz>
> 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: stable@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 <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>
> ---
> 
> 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,
> -				  &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)) {
> +		UDRS->last_checked = 0;

UDRS will still break the compilation here.

> +		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;
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
  2021-01-21 15:02                           ` Denis Efremov
@ 2021-01-21 15:05                             ` Jiri Kosina
  2021-01-22 11:13                               ` [PATCH] floppy: reintroduce O_NDELAY fix Jiri Kosina
  0 siblings, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2021-01-21 15:05 UTC (permalink / raw)
  To: Denis Efremov; +Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt

On Thu, 21 Jan 2021, Denis Efremov wrote:

> > From: Jiri Kosina <jkosina@suse.cz>
> > 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: stable@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


^ permalink raw reply	[flat|nested] 33+ messages in thread
* [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-21 15:05                             ` Jiri Kosina
@ 2021-01-22 11:13                               ` Jiri Kosina
  2021-01-26  8:21                                 ` Denis Efremov
  0 siblings, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2021-01-22 11:13 UTC (permalink / raw)
  To: Denis Efremov, Jens Axboe
  Cc: linux-kernel, linux-block, Wim Osterholt, Kurt Garloff

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>
---
 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;

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply related	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-22 11:13                               ` [PATCH] floppy: reintroduce O_NDELAY fix Jiri Kosina
@ 2021-01-26  8:21                                 ` Denis Efremov
  2021-01-26  9:31                                   ` Kurt Garloff
  2021-02-04  9:24                                   ` Jiri Kosina
  0 siblings, 2 replies; 33+ messages in thread
From: Denis Efremov @ 2021-01-26  8:21 UTC (permalink / raw)
  To: Jiri Kosina, Jens Axboe
  Cc: linux-kernel, linux-block, Wim Osterholt, Kurt Garloff



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;
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-26  8:21                                 ` Denis Efremov
@ 2021-01-26  9:31                                   ` Kurt Garloff
  2021-01-26  9:59                                     ` Denis Efremov
  2021-02-04  9:24                                   ` Jiri Kosina
  1 sibling, 1 reply; 33+ messages in thread
From: Kurt Garloff @ 2021-01-26  9:31 UTC (permalink / raw)
  To: Denis Efremov, Jiri Kosina, Jens Axboe
  Cc: linux-kernel, linux-block, Wim Osterholt

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,

-- 
Kurt Garloff <kurt@garloff.de>, Cologne, Germany



^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-26  9:31                                   ` Kurt Garloff
@ 2021-01-26  9:59                                     ` Denis Efremov
  0 siblings, 0 replies; 33+ messages in thread
From: Denis Efremov @ 2021-01-26  9:59 UTC (permalink / raw)
  To: Kurt Garloff, Jiri Kosina, Jens Axboe
  Cc: linux-kernel, linux-block, Wim Osterholt



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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-01-26  8:21                                 ` Denis Efremov
  2021-01-26  9:31                                   ` Kurt Garloff
@ 2021-02-04  9:24                                   ` Jiri Kosina
  2021-02-04 10:18                                     ` Denis Efremov
  1 sibling, 1 reply; 33+ messages in thread
From: Jiri Kosina @ 2021-02-04  9:24 UTC (permalink / raw)
  To: Denis Efremov
  Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt, Kurt Garloff

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,

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH] floppy: reintroduce O_NDELAY fix
  2021-02-04  9:24                                   ` Jiri Kosina
@ 2021-02-04 10:18                                     ` Denis Efremov
  0 siblings, 0 replies; 33+ messages in thread
From: Denis Efremov @ 2021-02-04 10:18 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jens Axboe, linux-kernel, linux-block, Wim Osterholt, Kurt Garloff



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

^ permalink raw reply	[flat|nested] 33+ messages in thread
end of thread, other threads:[~2021-02-04 10:19 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 23:02 disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
2016-06-11 13:15 ` VFS regression ? " One Thousand Gnomes
2016-06-12  0:23   ` Wim Osterholt
2016-06-13 12:15 ` Jiri Kosina
2016-06-14 18:43   ` Wim Osterholt
2016-06-15  7:09     ` Jiri Kosina
2016-06-15 11:42       ` Wim Osterholt
2016-06-15 13:20       ` Al Viro
2016-06-15 14:13         ` Jiri Kosina
2016-06-15 22:47           ` Wim Osterholt
2016-06-16  7:53             ` [PATCH] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
2016-06-30 11:18               ` [PATCH RESEND] " Jiri Kosina
2016-07-25 18:09                 ` Wim Osterholt
2016-07-25 20:48                 ` Jens Axboe
2021-01-19 15:53                   ` Jiri Kosina
2021-01-21  4:44                     ` Denis Efremov
2021-01-21 10:25                       ` Jiri Kosina
2021-01-21 13:28                         ` [PATCH] floppy: reintroduce O_NDELAY fix kernel test robot
2021-01-21 13:28                           ` kernel test robot
2021-01-21 14:44                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Jiri Kosina
2021-01-21 15:02                           ` Denis Efremov
2021-01-21 15:05                             ` Jiri Kosina
2021-01-22 11:13                               ` [PATCH] floppy: reintroduce O_NDELAY fix Jiri Kosina
2021-01-26  8:21                                 ` Denis Efremov
2021-01-26  9:31                                   ` Kurt Garloff
2021-01-26  9:59                                     ` Denis Efremov
2021-02-04  9:24                                   ` Jiri Kosina
2021-02-04 10:18                                     ` Denis Efremov
2021-01-21 14:45                         ` [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open Denis Efremov
2016-06-15 23:07           ` disfunctional floppy driver in kernels 4.5, 4.6 and 4.7 Wim Osterholt
2016-06-15 23:12             ` Jiri Kosina
2016-06-15 23:13             ` Joe Perches
2016-06-14 19:09   ` Al Viro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.