All of lore.kernel.org
 help / color / mirror / Atom feed
From: Meng Xu <mengxu.gatech@gmail.com>
To: aacraid@microsemi.com, jejb@linux.vnet.ibm.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Cc: Meng Xu <mengxu.gatech@gmail.com>
Subject: [PATCH] aacraid: fix potential double-fetch issue
Date: Wed, 20 Sep 2017 00:08:52 -0400	[thread overview]
Message-ID: <1505880532-25847-1-git-send-email-mengxu.gatech@gmail.com> (raw)

While examining the kernel source code, I found a dangerous operation
that could turn into a double-fetch situation (a race condition bug)
where the same userspace memory region are fetched twice into kernel
with sanity checks after the first fetch while missing checks after the
second fetch.

1. The first userspace fetch happens in
copy_from_user(&fibsize, &user_srb->count,sizeof(u32))

2. Subsequently fibsize undergoes a few sanity checks and is then
used to allocate user_srbcmd = kmalloc(fibsize, GFP_KERNEL).

3. The second userspace fetch happens in
copy_from_user(user_srbcmd, user_srb,fibsize)

4. Given that the memory region pointed by user_srb can be fully
controlled in userspace, an attacker can race to override the
user_srb->count to arbitrary value (say 0XFFFFFFFF) after the first
fetch but before the second. The changed value will be copied to
user_srbcmd.

The patch explicitly overrides user_srbcmd->count after the second
userspace fetch with the value fibsize from the first userspace fetch.
In this way, it is assured that the relation, user_srbcmd->count stores
the size of the user_srbcmd buffer, still holds after the second fetch.

Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
---
 drivers/scsi/aacraid/commctrl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 9ab0fa9..734bf11 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -540,6 +540,12 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 		goto cleanup;
 	}
 
+	/* 
+	 * re-establish the relation that user_srbcmd->count holds the 
+	 * size of user_srbcmd 
+	 */
+	user_srbcmd->count = fibsize;
+
 	flags = user_srbcmd->flags; /* from user in cpu order */
 	switch (flags & (SRB_DataIn | SRB_DataOut)) {
 	case SRB_DataOut:
-- 
2.7.4

                 reply	other threads:[~2017-09-20  4:09 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=1505880532-25847-1-git-send-email-mengxu.gatech@gmail.com \
    --to=mengxu.gatech@gmail.com \
    --cc=aacraid@microsemi.com \
    --cc=jejb@linux.vnet.ibm.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.