PATCH: scsi: make scsi reset permissions more relaxed (RFC)

PATCH: scsi: make scsi reset permissions more relaxed (RFC)

[Marcus Meissner]

Hi folks,

cdrecord wants to whack the CD drive with a SCSI RESET ...

So far SCSI RESET can be done at 4 levels (target, device, bus, host)
and all 4 are checked for CAP_SYS_ADMIN / CAP_SYS_RAWIO.


As the cdrecord author wants special permissions for cdrecord, readcd ,
cdda2wav to allow it to send SCSI RESET commands I was wondering if
relaxing the permission is a potential idea?

This would allow SCSI reset on target/device if a local user
gets regular access to a SCSI device (via udev acls etc.)


(I know that the actual reset code will fall back into the chain
 target -> device -> bus -> host resetting if one fails.)

Signed-off-by: Marcus Meissner <meissner@suse.de>

Ciao, Marcus
---
 drivers/scsi/scsi_ioctl.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index d9564fb..770720e 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -306,22 +306,26 @@ int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
 			return 0;
 		switch (val) {
 		case SG_SCSI_RESET_DEVICE:
+			/* allowed if you can send scsi commands to the device */
 			val = SCSI_TRY_RESET_DEVICE;
 			break;
 		case SG_SCSI_RESET_TARGET:
+			/* allowed if you can send scsi commands to the device */
 			val = SCSI_TRY_RESET_TARGET;
 			break;
 		case SG_SCSI_RESET_BUS:
+			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
+				return -EACCES;
 			val = SCSI_TRY_RESET_BUS;
 			break;
 		case SG_SCSI_RESET_HOST:
+			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
+				return -EACCES;
 			val = SCSI_TRY_RESET_HOST;
 			break;
 		default:
 			return -EINVAL;
 		}
-		if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
-			return -EACCES;
 		return (scsi_reset_provider(sdev, val) ==
 			SUCCESS) ? 0 : -EIO;
 	}
-- 
1.8.1.4

Re: PATCH: scsi: make scsi reset permissions more relaxed (RFC)

[Hannes Reinecke]

On 08/30/2013 02:04 PM, Marcus Meissner wrote:
> Hi folks,
> 
> cdrecord wants to whack the CD drive with a SCSI RESET ...
> 
> So far SCSI RESET can be done at 4 levels (target, device, bus, host)
> and all 4 are checked for CAP_SYS_ADMIN / CAP_SYS_RAWIO.
> 
> 
> As the cdrecord author wants special permissions for cdrecord, readcd ,
> cdda2wav to allow it to send SCSI RESET commands I was wondering if
> relaxing the permission is a potential idea?
> 
> This would allow SCSI reset on target/device if a local user
> gets regular access to a SCSI device (via udev acls etc.)
> 
Hmm. Device and target reset are typically implemented at the
transport level (SCSI itself doesn't have any commands for this).
So it should be possible to inject them via bsg, and as such should
have the same restrictions as bsg has.
Assuming that sg and bsg follow the same rules the patch should be okay.

> 
> (I know that the actual reset code will fall back into the chain
>  target -> device -> bus -> host resetting if one fails.)
> 
Actually, it doesn't. sg_reset_provider() will just call the
individual function.

If the function fails _and_ there are commands send to the device
then the error handling will kick in, but it won't be triggered
directly.
Not sure if that makes a difference, though.

> Signed-off-by: Marcus Meissner <meissner@suse.de>
> 
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: PATCH: scsi: make scsi reset permissions more relaxed (RFC)

[Douglas Gilbert]

On 13-08-30 02:04 PM, Marcus Meissner wrote:
> Hi folks,
>
> cdrecord wants to whack the CD drive with a SCSI RESET ...
>
> So far SCSI RESET can be done at 4 levels (target, device, bus, host)
> and all 4 are checked for CAP_SYS_ADMIN / CAP_SYS_RAWIO.
>
>
> As the cdrecord author wants special permissions for cdrecord, readcd ,
> cdda2wav to allow it to send SCSI RESET commands I was wondering if
> relaxing the permission is a potential idea?
>
> This would allow SCSI reset on target/device if a local user
> gets regular access to a SCSI device (via udev acls etc.)
>
>
> (I know that the actual reset code will fall back into the chain
>   target -> device -> bus -> host resetting if one fails.)

Hi,
That escalation sequence probably should be:
    device(LU) -> target -> bus -> host

I proposed the following patch some time back to give the
user space finer resolution on resets with the option of
stopping the escalation but it has gone nowhere:
   http://marc.info/?l=linux-scsi&m=136104139102048&w=2

With that patch you might only allow an unprivileged user
the non-escalating LU and target reset variants.

If changes are made in that area, we might like to think
about adding a new RESET variant mapping through to the I_T
Nexus Reset TMF.

Doug Gilbert

> ---
>   drivers/scsi/scsi_ioctl.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
> index d9564fb..770720e 100644
> --- a/drivers/scsi/scsi_ioctl.c
> +++ b/drivers/scsi/scsi_ioctl.c
> @@ -306,22 +306,26 @@ int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
>   			return 0;
>   		switch (val) {
>   		case SG_SCSI_RESET_DEVICE:
> +			/* allowed if you can send scsi commands to the device */
>   			val = SCSI_TRY_RESET_DEVICE;
>   			break;
>   		case SG_SCSI_RESET_TARGET:
> +			/* allowed if you can send scsi commands to the device */
>   			val = SCSI_TRY_RESET_TARGET;
>   			break;
>   		case SG_SCSI_RESET_BUS:
> +			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> +				return -EACCES;
>   			val = SCSI_TRY_RESET_BUS;
>   			break;
>   		case SG_SCSI_RESET_HOST:
> +			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> +				return -EACCES;
>   			val = SCSI_TRY_RESET_HOST;
>   			break;
>   		default:
>   			return -EINVAL;
>   		}
> -		if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> -			return -EACCES;
>   		return (scsi_reset_provider(sdev, val) ==
>   			SUCCESS) ? 0 : -EIO;
>   	}
>

Re: PATCH: scsi: make scsi reset permissions more relaxed (RFC)

[Jeremy Linton]

On 8/30/2013 7:47 AM, Douglas Gilbert wrote:

> I proposed the following patch some time back to give the user space finer
> resolution on resets with the option of stopping the escalation but it has
> gone nowhere: http://marc.info/?l=linux-scsi&m=136104139102048&w=2
> 
> With that patch you might only allow an unprivileged user the
> non-escalating LU and target reset variants.
> 
> If changes are made in that area, we might like to think about adding a new
> RESET variant mapping through to the I_T Nexus Reset TMF.

And a fine, incredibly useful patch it is. To the point of basically being a
requirement for SAN environments. Without it, all kinds of havoc can ensue.

But, the problem of burners going out to lunch, shows why its a stopgap. As most
burners are going to be SATA attached (without an expander), you probably want
to escalate all the way to the host reset if none of the other options work.

With a few other tweaks and the no-escalate patch, its possible to implement
escalation logic outside of the kernel that is aware of the device topology and
individual device states. That way HBA's aren't getting reset under active
functional devices.