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

From: Denis Efremov <efremov@linux.com>
To: Jiri Kosina <jikos@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
	Wim Osterholt <wim@djo.tudelft.nl>
Subject: Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
Date: Thu, 21 Jan 2021 18:02:48 +0300	[thread overview]
Message-ID: <e503292b-5f51-eac5-771f-e35991d1084c@linux.com> (raw)
In-Reply-To: <nycvar.YFH.7.76.2101211543230.5622@cbobk.fhfr.pm>

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