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

2017-08-11 Thread Greg Kroah-Hartman 
On Fri, Aug 11, 2017 at 08:30:45AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 11, 2017 at 09:14:09AM +0200, Johannes Thumshirn wrote:
> > On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> > > else between it and the newer one?  Drop this and you provide a
> > > backport?  Something else?
> > > 
> > > Totally confusing...
> > 
> > Sorry for confusing you, here's the backport for your 4.12 stable branch,
> 
> Ok, I've dropped the original patch here now and will queue this one up
> for the next round of stable updates...

Now applied, thanks.

greg k-h

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

2017-08-11 Thread Greg Kroah-Hartman 
On Fri, Aug 11, 2017 at 08:30:45AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 11, 2017 at 09:14:09AM +0200, Johannes Thumshirn wrote:
> > On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> > > else between it and the newer one?  Drop this and you provide a
> > > backport?  Something else?
> > > 
> > > Totally confusing...
> > 
> > Sorry for confusing you, here's the backport for your 4.12 stable branch,
> 
> Ok, I've dropped the original patch here now and will queue this one up
> for the next round of stable updates...

Now applied, thanks.

greg k-h

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

2017-08-11 Thread Greg Kroah-Hartman 
On Fri, Aug 11, 2017 at 09:14:09AM +0200, Johannes Thumshirn wrote:
> On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> > else between it and the newer one?  Drop this and you provide a
> > backport?  Something else?
> > 
> > Totally confusing...
> 
> Sorry for confusing you, here's the backport for your 4.12 stable branch,

Ok, I've dropped the original patch here now and will queue this one up
for the next round of stable updates...

thanks,

greg k-h

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

2017-08-11 Thread Greg Kroah-Hartman 
On Fri, Aug 11, 2017 at 09:14:09AM +0200, Johannes Thumshirn wrote:
> On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> > else between it and the newer one?  Drop this and you provide a
> > backport?  Something else?
> > 
> > Totally confusing...
> 
> Sorry for confusing you, here's the backport for your 4.12 stable branch,

Ok, I've dropped the original patch here now and will queue this one up
for the next round of stable updates...

thanks,

greg k-h

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

2017-08-11 Thread Johannes Thumshirn 
On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> else between it and the newer one?  Drop this and you provide a
> backport?  Something else?
> 
> Totally confusing...

Sorry for confusing you, here's the backport for your 4.12 stable branch,

Thanks,
Johannes

>From 469957522c4b356a313cb369e3e14fdac104370f Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn 
Date: Thu, 27 Jul 2017 09:11:26 +0200
Subject: [PATCH] scsi: sg: only check for dxfer_len greater than 256M

commit f930c7043663188429cd9b254e9d761edfc101ce upstream

Don't make any assumptions on the sg_io_hdr_t::dxfer_direction or the
sg_io_hdr_t::dxferp in order to determine if it is a valid request. The
only way we can check for bad requests is by checking if the length
exceeds 256M.

Signed-off-by: Johannes Thumshirn 
Fixes: 28676d869bbb (scsi: sg: check for valid direction before starting the
request)
Reported-by: Jason L Tibbitts III 
Tested-by: Jason L Tibbitts III 
Suggested-by: Doug Gilbert 
Cc: Doug Gilbert 
Cc: 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Martin K. Petersen 
---
 drivers/scsi/sg.c | 25 +
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 82c33a6edbea..aa6f1debeaa7 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -751,29 +751,6 @@ sg_new_write(Sg_fd *sfp, struct file *file, const char 
__user *buf,
return count;
 }
 
-static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
-{
-   switch (hp->dxfer_direction) {
-   case SG_DXFER_NONE:
-   if (hp->dxferp || hp->dxfer_len > 0)
-   return false;
-   return true;
-   case SG_DXFER_TO_DEV:
-   case SG_DXFER_FROM_DEV:
-   case SG_DXFER_TO_FROM_DEV:
-   if (!hp->dxferp || hp->dxfer_len == 0)
-   return false;
-   return true;
-   case SG_DXFER_UNKNOWN:
-   if ((!hp->dxferp && hp->dxfer_len) ||
-   (hp->dxferp && hp->dxfer_len == 0))
-   return false;
-   return true;
-   default:
-   return false;
-   }
-}
-
 static int
 sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking)
@@ -794,7 +771,7 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
"sg_common_write:  scsi opcode=0x%02x, cmd_size=%d\n",
(int) cmnd[0], (int) hp->cmd_len));
 
-   if (!sg_is_valid_dxfer(hp))
+   if (hp->dxfer_len >= SZ_256M)
return -EINVAL;
 
k = sg_start_req(srp, cmnd);
-- 
2.12.3


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

2017-08-11 Thread Johannes Thumshirn 
On Thu, Aug 10, 2017 at 08:11:34AM -0700, Greg KH wrote:
> else between it and the newer one?  Drop this and you provide a
> backport?  Something else?
> 
> Totally confusing...

Sorry for confusing you, here's the backport for your 4.12 stable branch,

Thanks,
Johannes

>From 469957522c4b356a313cb369e3e14fdac104370f Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn 
Date: Thu, 27 Jul 2017 09:11:26 +0200
Subject: [PATCH] scsi: sg: only check for dxfer_len greater than 256M

commit f930c7043663188429cd9b254e9d761edfc101ce upstream

Don't make any assumptions on the sg_io_hdr_t::dxfer_direction or the
sg_io_hdr_t::dxferp in order to determine if it is a valid request. The
only way we can check for bad requests is by checking if the length
exceeds 256M.

Signed-off-by: Johannes Thumshirn 
Fixes: 28676d869bbb (scsi: sg: check for valid direction before starting the
request)
Reported-by: Jason L Tibbitts III 
Tested-by: Jason L Tibbitts III 
Suggested-by: Doug Gilbert 
Cc: Doug Gilbert 
Cc: 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Martin K. Petersen 
---
 drivers/scsi/sg.c | 25 +
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 82c33a6edbea..aa6f1debeaa7 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -751,29 +751,6 @@ sg_new_write(Sg_fd *sfp, struct file *file, const char 
__user *buf,
return count;
 }
 
-static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
-{
-   switch (hp->dxfer_direction) {
-   case SG_DXFER_NONE:
-   if (hp->dxferp || hp->dxfer_len > 0)
-   return false;
-   return true;
-   case SG_DXFER_TO_DEV:
-   case SG_DXFER_FROM_DEV:
-   case SG_DXFER_TO_FROM_DEV:
-   if (!hp->dxferp || hp->dxfer_len == 0)
-   return false;
-   return true;
-   case SG_DXFER_UNKNOWN:
-   if ((!hp->dxferp && hp->dxfer_len) ||
-   (hp->dxferp && hp->dxfer_len == 0))
-   return false;
-   return true;
-   default:
-   return false;
-   }
-}
-
 static int
 sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking)
@@ -794,7 +771,7 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
"sg_common_write:  scsi opcode=0x%02x, cmd_size=%d\n",
(int) cmnd[0], (int) hp->cmd_len));
 
-   if (!sg_is_valid_dxfer(hp))
+   if (hp->dxfer_len >= SZ_256M)
return -EINVAL;
 
k = sg_start_req(srp, cmnd);
-- 
2.12.3


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

2017-08-10 Thread Greg Kroah-Hartman 
On Thu, Aug 10, 2017 at 08:14:14AM +0200, Johannes Thumshirn wrote:
> On Wed, Aug 09, 2017 at 09:51:48AM -0700, Greg KH wrote:
> > 4.12-stable review patch.  If anyone has any objections, please let me know.
> 
> Yep I do, please don't use this one. It has a follow up/was superseded by:
> f930c7043663 ("scsi: sg: only check for dxfer_len greater than 256M")
> in Linus' tree
> 
> (Yes I know I explicitly sent it to stable, mea culpa)

"superseeded" implies that this patch needs to be applied in order for
the "new" one to work properly.  So I tried applying it, but it doesn't
apply cleanly.  Then I tried by removing this patch and then applying
the new one.  That too didn't work at all.

So, what am I supposed to do here?  Leave this in, and apply something
else between it and the newer one?  Drop this and you provide a
backport?  Something else?

Totally confusing...

greg k-h

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

2017-08-10 Thread Greg Kroah-Hartman 
On Thu, Aug 10, 2017 at 08:14:14AM +0200, Johannes Thumshirn wrote:
> On Wed, Aug 09, 2017 at 09:51:48AM -0700, Greg KH wrote:
> > 4.12-stable review patch.  If anyone has any objections, please let me know.
> 
> Yep I do, please don't use this one. It has a follow up/was superseded by:
> f930c7043663 ("scsi: sg: only check for dxfer_len greater than 256M")
> in Linus' tree
> 
> (Yes I know I explicitly sent it to stable, mea culpa)

"superseeded" implies that this patch needs to be applied in order for
the "new" one to work properly.  So I tried applying it, but it doesn't
apply cleanly.  Then I tried by removing this patch and then applying
the new one.  That too didn't work at all.

So, what am I supposed to do here?  Leave this in, and apply something
else between it and the newer one?  Drop this and you provide a
backport?  Something else?

Totally confusing...

greg k-h

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

2017-08-10 Thread 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 
> 
> 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 
> Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the 
> request")
> Reported-by: Chris Clayton 
> Tested-by: Chris Clayton 
> Cc: Douglas Gilbert 
> Reviewed-by: Hannes Reinecke 
> Tested-by: Chris Clayton 
> Acked-by: Douglas Gilbert 
> Signed-off-by: Martin K. Petersen 
> Signed-off-by: Greg Kroah-Hartman 
> 
> ---
>  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;
> 
>

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

2017-08-10 Thread 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 
> 
> 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 
> Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the 
> request")
> Reported-by: Chris Clayton 
> Tested-by: Chris Clayton 
> Cc: Douglas Gilbert 
> Reviewed-by: Hannes Reinecke 
> Tested-by: Chris Clayton 
> Acked-by: Douglas Gilbert 
> Signed-off-by: Martin K. Petersen 
> Signed-off-by: Greg Kroah-Hartman 
> 
> ---
>  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;
> 
>

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

2017-08-10 Thread Johannes Thumshirn 
On Wed, Aug 09, 2017 at 09:51:48AM -0700, Greg KH wrote:
> 4.12-stable review patch.  If anyone has any objections, please let me know.

Yep I do, please don't use this one. It has a follow up/was superseded by:
f930c7043663 ("scsi: sg: only check for dxfer_len greater than 256M")
in Linus' tree

(Yes I know I explicitly sent it to stable, mea culpa)

Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

2017-08-10 Thread Johannes Thumshirn 
On Wed, Aug 09, 2017 at 09:51:48AM -0700, Greg KH wrote:
> 4.12-stable review patch.  If anyone has any objections, please let me know.

Yep I do, please don't use this one. It has a follow up/was superseded by:
f930c7043663 ("scsi: sg: only check for dxfer_len greater than 256M")
in Linus' tree

(Yes I know I explicitly sent it to stable, mea culpa)

Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

2017-08-09 Thread Greg Kroah-Hartman 
4.12-stable review patch.  If anyone has any objections, please let me know.

--

From: Johannes Thumshirn 

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 
Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the 
request")
Reported-by: Chris Clayton 
Tested-by: Chris Clayton 
Cc: Douglas Gilbert 
Reviewed-by: Hannes Reinecke 
Tested-by: Chris Clayton 
Acked-by: Douglas Gilbert 
Signed-off-by: Martin K. Petersen 
Signed-off-by: Greg Kroah-Hartman 

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

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

2017-08-09 Thread Greg Kroah-Hartman 
4.12-stable review patch.  If anyone has any objections, please let me know.

--

From: Johannes Thumshirn 

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 
Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the 
request")
Reported-by: Chris Clayton 
Tested-by: Chris Clayton 
Cc: Douglas Gilbert 
Reviewed-by: Hannes Reinecke 
Tested-by: Chris Clayton 
Acked-by: Douglas Gilbert 
Signed-off-by: Martin K. Petersen 
Signed-off-by: Greg Kroah-Hartman 

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