linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wenwen Wang <wang6495@umn.edu>
To: Wenwen Wang <wang6495@umn.edu>
Cc: Kangjie Lu <kjlu@umn.edu>, Doug Gilbert <dgilbert@interlog.com>,
	"James E.J. Bottomley" <jejb@linux.vnet.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org (open list:SCSI SG DRIVER),
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] scsi: sg: fix a missing-check bug
Date: Sat,  5 May 2018 22:21:34 -0500	[thread overview]
Message-ID: <1525576895-15708-1-git-send-email-wang6495@umn.edu> (raw)

In sg_write(), the opcode of the command is firstly copied from the
userspace pointer 'buf' and saved to the kernel variable 'opcode', using
the __get_user() function. The size of the command, i.e., 'cmd_size' is
then calculated based on the 'opcode'. After that, the whole command,
including the opcode, is copied again from 'buf' using the
__copy_from_user() function and saved to 'cmnd'. Finally, the function
 sg_common_write() is invoked to process 'cmnd'. Given that the 'buf'
pointer resides in userspace, a malicious userspace process can race to
change the opcode of the command between the two copies. That means, the
opcode indicated by the variable 'opcode' could be different from the
opcode in 'cmnd'. This can cause inconsistent data in 'cmnd' and
potential logical errors in the function sg_common_write(), as it needs to
work on 'cmnd'.

This patch reuses the opcode obtained in the first copy and only copies the
remaining part of the command from userspace.

Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
 drivers/scsi/sg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c198b963..0ad8106 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -657,7 +657,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
 	hp->flags = input_size;	/* structure abuse ... */
 	hp->pack_id = old_hdr.pack_id;
 	hp->usr_ptr = NULL;
-	if (__copy_from_user(cmnd, buf, cmd_size))
+	cmnd[0] = opcode;
+	if (__copy_from_user(cmnd + 1, buf + 1, cmd_size - 1))
 		return -EFAULT;
 	/*
 	 * SG_DXFER_TO_FROM_DEV is functionally equivalent to SG_DXFER_FROM_DEV,
-- 
2.7.4

             reply	other threads:[~2018-05-06  3:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06  3:21 Wenwen Wang [this message]
2018-05-07  5:13 ` [PATCH] scsi: sg: fix a missing-check bug Douglas Gilbert
2018-05-18 21:48   ` Wenwen Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1525576895-15708-1-git-send-email-wang6495@umn.edu \
    --to=wang6495@umn.edu \
    --cc=dgilbert@interlog.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=kjlu@umn.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).