linux-kernel - Re: [PATCH] scsi: clear UAC before sending SG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6baf4adf-e97b-9584-1aae-d12235c362fe@interlog.com>
Date:   Thu, 10 Sep 2020 13:43:49 -0400
From:   Douglas Gilbert <dgilbert@...erlog.com>
To:     Randall Huang <huangrandall@...gle.com>, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scsi: clear UAC before sending SG_IO

On 2020-09-10 6:15 a.m., Randall Huang wrote:
> Make sure UAC is clear before sending SG_IO.
> 
> Signed-off-by: Randall Huang <huangrandall@...gle.com>

This patch just looks wrong. Imagine if every LLD front loaded some LLD
specific code before each invocation of ioctl(SG_IO). Is UAC Unit Attention
Condition? If so the mid-level notes them as they fly past.

Haven't looked at the rest of the patchset but I suspect the "wlun_clr_uac"
work needs a rethink. If that is the REPORT LUNS Well known LUN then perhaps
it could be handled in the mid-level scanning code. Otherwise it should
be handled in the LLD/UFS.

Also users of ioctl(SG_IO) should be capable of handling UAs, even if they
are irrelevant, and repeat the invocation. Finally ioctl(sg_dev, SG_IO) is
not the only way to send a pass-through command, there are also
   - write(sg_dev, ...)
   - ioctl(bsg_dev, SG_IO, ...)
   - ioctl(most_blk_devs, SG_IO, ...)
   - ioctl(st_dev, SG_IO, ...)

Hopefully I have convinced you by now not to take this route.

Doug Gilbert

> ---
>   drivers/scsi/sg.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 20472aaaf630..ad11bca47ae8 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -922,6 +922,7 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>   	int result, val, read_only;
>   	Sg_request *srp;
>   	unsigned long iflags;
> +	int _cmd;
>   
>   	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
>   				   "sg_ioctl: cmd=0x%x\n", (int) cmd_in));
> @@ -933,6 +934,13 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>   			return -ENODEV;
>   		if (!scsi_block_when_processing_errors(sdp->device))
>   			return -ENXIO;
> +
> +		_cmd = SCSI_UFS_REQUEST_SENSE;
> +		if (sdp->device->host->wlun_clr_uac) {
> +			sdp->device->host->hostt->ioctl(sdp->device, _cmd, NULL);
> +			sdp->device->host->wlun_clr_uac = false;
> +		}
> +
>   		result = sg_new_write(sfp, filp, p, SZ_SG_IO_HDR,
>   				 1, read_only, 1, &srp);
>   		if (result < 0)
>