Re: [PATCH 4.12 004/106] scsi: sg: fix SG_DXFER_FROM_DEV transfers

[Chris Clayton]

On 09/08/17 17:51, Greg Kroah-Hartman wrote:
> 4.12-stable review patch.  If anyone has any objections, please let me know.
> 
> -------
 I repeat my comments when the patch was queued for stable:

1. Johannes' commit message says that the transfer must have a length bigger than 0, so the code should return false if
the length is less than or equal to 0, but the test is for less than 0.

2. But in any case, there's another patch that removes all this sg_is_valid_dxfer() jiggery-pokery and replaces it with
a simpler test. It hasn't reached Linus' tree yet but is, I believe, cc'd to stable.


As Johannes said in response to the second of my comments, the patch that replaces sg_is_valid_dxfer() with a simpler
test is now in Linus' tree - commit f930c7043663188429cd9b254e9d761edfc101ce. Without that change, I think there is
still some breakage in sg.

Chris
-----------
> 
> From: Johannes Thumshirn <jthumshirn@xxxxxxx>
> 
> commit 68c59fcea1f2c6a54c62aa896cc623c1b5bc9b47 upstream.
> 
> SG_DXFER_FROM_DEV transfers do not necessarily have a dxferp as we set
> it to NULL for the old sg_io read/write interface, but must have a
> length bigger than 0. This fixes a regression introduced by commit
> 28676d869bbb ("scsi: sg: check for valid direction before starting the
> request")
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the request")
> Reported-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx>
> Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx>
> Cc: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxxx>
> Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx>
> Acked-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
> Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> ---
>  drivers/scsi/sg.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -758,8 +758,11 @@ static bool sg_is_valid_dxfer(sg_io_hdr_
>  		if (hp->dxferp || hp->dxfer_len > 0)
>  			return false;
>  		return true;
> -	case SG_DXFER_TO_DEV:
>  	case SG_DXFER_FROM_DEV:
> +		if (hp->dxfer_len < 0)
> +			return false;
> +		return true;
> +	case SG_DXFER_TO_DEV:
>  	case SG_DXFER_TO_FROM_DEV:
>  		if (!hp->dxferp || hp->dxfer_len == 0)
>  			return false;
> 
>