Re: [PATCH] sg: fix dxferp in from_to case

[Martin K. Petersen]

> "Doug" == Douglas Gilbert  writes:

Doug> Make sure that a user space pointer is passed through when data
Doug> follows the sg_header structure and command.  Fix the abnormal
Doug> case when a non-zero reply_len is also given.

Applied to 4.5/scsi-fixes.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sg: fix dxferp in from_to case

[Ewan Milne]

On Thu, 2016-03-03 at 00:31 -0500, Douglas Gilbert wrote:
> This patch is in response to this report:
> http://www.spinics.net/lists/linux-scsi/msg93456.html
> 
> One of the strange things that the original sg driver did was let
> the user provide both a data-out buffer (it followed the
> sg_header+cdb) _and_ specify a reply length greater than zero. What
> happened was that the user data-out buffer was copied into some
> kernel buffers and then the mid level was told a read type operation
> would take place with the data from the device overwriting the same
> kernel buffers. The user would then read those kernel buffers back
> into the user space.
> 
>  From what I can tell, the above action was broken by a change in
> 2008 and syzkaller found that out recently.
> 
> ChangeLog:
>make sure that a user space pointer is passed through
>when data follows the sg_header structure and command.
>Fix the abnormal case when a non-zero reply_len is also
>given.
> 
> Signed-off-by: Douglas Gilbert 

This looks correct to me.  hp->dxferp used to be set unconditionally,
but commit fad7f01e changed it to only be set in the SG_DXFER_TO_DEV
case.

Reviewed-by: Ewan D. Milne 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sg: fix dxferp in from_to case

[Martin K. Petersen]

> "Doug" == Douglas Gilbert  writes:

Doug> This patch is in response to this report:
Doug> http://www.spinics.net/lists/linux-scsi/msg93456.html

Doug> One of the strange things that the original sg driver did was let
Doug> the user provide both a data-out buffer (it followed the
Doug> sg_header+cdb) _and_ specify a reply length greater than
Doug> zero. What happened was that the user data-out buffer was copied
Doug> into some kernel buffers and then the mid level was told a read
Doug> type operation would take place with the data from the device
Doug> overwriting the same kernel buffers. The user would then read
Doug> those kernel buffers back into the user space.

Doug> From what I can tell, the above action was broken by a change in
Doug> 2008 and syzkaller found that out recently.

Doug>ChangeLog: make sure that a user space pointer is passed
Doug> through when data follows the sg_header structure and command.
Doug> Fix the abnormal case when a non-zero reply_len is also given.

Somebody please review.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sg: fix dxferp in from_to case

[Douglas Gilbert]

This patch is in response to this report:
   http://www.spinics.net/lists/linux-scsi/msg93456.html

One of the strange things that the original sg driver did was let
the user provide both a data-out buffer (it followed the
sg_header+cdb) _and_ specify a reply length greater than zero. What
happened was that the user data-out buffer was copied into some
kernel buffers and then the mid level was told a read type operation
would take place with the data from the device overwriting the same
kernel buffers. The user would then read those kernel buffers back
into the user space.

From what I can tell, the above action was broken by a change in
2008 and syzkaller found that out recently.

   ChangeLog:
  make sure that a user space pointer is passed through
  when data follows the sg_header structure and command.
  Fix the abnormal case when a non-zero reply_len is also
  given.

Signed-off-by: Douglas Gilbert 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5e82067..ae7d9bd 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -652,7 +652,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
 	else
 		hp->dxfer_direction = (mxsize > 0) ? SG_DXFER_FROM_DEV : SG_DXFER_NONE;
 	hp->dxfer_len = mxsize;
-	if (hp->dxfer_direction == SG_DXFER_TO_DEV)
+	if ((hp->dxfer_direction == SG_DXFER_TO_DEV) ||
+	(hp->dxfer_direction == SG_DXFER_TO_FROM_DEV))
 		hp->dxferp = (char __user *)buf + cmd_size;
 	else
 		hp->dxferp = NULL;