From: Arnd Bergmann <arnd@arndb.de>
To: Kashyap Desai <kashyap.desai@broadcom.com>,
Sumit Saxena <sumit.saxena@broadcom.com>,
Shivasharan S <shivasharan.srikanteshwara@broadcom.com>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: hch@infradead.org, Arnd Bergmann <arnd@arndb.de>,
Anand Lodnoor <anand.lodnoor@broadcom.com>,
Chandrakanth Patil <chandrakanth.patil@broadcom.com>,
Hannes Reinecke <hare@suse.de>,
megaraidlinux.pdl@broadcom.com, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 3/3] scsi: megaraid_sas: simplify compat_ioctl handling
Date: Tue, 8 Sep 2020 23:36:23 +0200 [thread overview]
Message-ID: <20200908213715.3553098-3-arnd@arndb.de> (raw)
In-Reply-To: <20200908213715.3553098-1-arnd@arndb.de>
There have been several attempts to fix serious problems
in the compat handling in megasas_mgmt_compat_ioctl_fw(),
and it also uses the compat_alloc_user_space() function.
Folding the compat handling into the regular ioctl
function with in_compat_syscall() simplifies it a lot and
avoids some of the remaining problems:
- missing handling of unaligned pointers
- overflowing the ioc->frame.raw array from
invalid input
- compat_alloc_user_space()
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/scsi/megaraid/megaraid_sas_base.c | 123 ++++++++++------------
1 file changed, 56 insertions(+), 67 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index c3de69f3bee8..601dab468823 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -8331,6 +8331,56 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
return error;
}
+static struct megasas_iocpacket *megasas_compat_iocpacket_get_user(void __user *arg)
+{
+ int err = -EFAULT;
+#ifdef CONFIG_COMPAT
+ struct megasas_iocpacket *ioc;
+ struct compat_megasas_iocpacket __user *cioc = arg;
+ int i;
+
+ ioc = kzalloc(sizeof(*ioc), GFP_KERNEL);
+ if (copy_from_user(ioc, arg,
+ offsetof(struct megasas_iocpacket, frame) + 128))
+ goto out;
+
+ /*
+ * The sense_ptr is used in megasas_mgmt_fw_ioctl only when
+ * sense_len is not null, so prepare the 64bit value under
+ * the same condition.
+ */
+ if (ioc->sense_len) {
+ compat_uptr_t *sense_ioc_ptr;
+ void __user *sense_cioc;
+
+ /* make sure the pointer is inside of frame.raw */
+ if (ioc->sense_off >
+ (sizeof(ioc->frame.raw) - sizeof(void __user*))) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ sense_ioc_ptr = (compat_uptr_t *)&ioc->frame.raw[ioc->sense_off];
+ sense_cioc = compat_ptr(get_unaligned(sense_ioc_ptr));
+ put_unaligned((unsigned long)sense_cioc, (void **)sense_ioc_ptr);
+ }
+
+ for (i = 0; i < MAX_IOCTL_SGE; i++) {
+ compat_uptr_t iov_base;
+ if (get_user(iov_base, &cioc->sgl[i].iov_base) ||
+ get_user(ioc->sgl[i].iov_len, &cioc->sgl[i].iov_len)) {
+ goto out;
+ }
+ ioc->sgl[i].iov_base = compat_ptr(iov_base);
+ }
+
+ return ioc;
+out:
+ kfree(ioc);
+#endif
+ return ERR_PTR(err);
+}
+
static int megasas_mgmt_ioctl_fw(struct file *file, unsigned long arg)
{
struct megasas_iocpacket __user *user_ioc =
@@ -8339,7 +8389,11 @@ static int megasas_mgmt_ioctl_fw(struct file *file, unsigned long arg)
struct megasas_instance *instance;
int error;
- ioc = memdup_user(user_ioc, sizeof(*ioc));
+ if (in_compat_syscall())
+ ioc = megasas_compat_iocpacket_get_user(user_ioc);
+ else
+ ioc = memdup_user(user_ioc, sizeof(struct megasas_iocpacket));
+
if (IS_ERR(ioc))
return PTR_ERR(ioc);
@@ -8444,78 +8498,13 @@ megasas_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
#ifdef CONFIG_COMPAT
-static int megasas_mgmt_compat_ioctl_fw(struct file *file, unsigned long arg)
-{
- struct compat_megasas_iocpacket __user *cioc =
- (struct compat_megasas_iocpacket __user *)arg;
- struct megasas_iocpacket __user *ioc =
- compat_alloc_user_space(sizeof(struct megasas_iocpacket));
- int i;
- int error = 0;
- compat_uptr_t ptr;
- u32 local_sense_off;
- u32 local_sense_len;
- u32 user_sense_off;
-
- if (clear_user(ioc, sizeof(*ioc)))
- return -EFAULT;
-
- if (copy_in_user(&ioc->host_no, &cioc->host_no, sizeof(u16)) ||
- copy_in_user(&ioc->sgl_off, &cioc->sgl_off, sizeof(u32)) ||
- copy_in_user(&ioc->sense_off, &cioc->sense_off, sizeof(u32)) ||
- copy_in_user(&ioc->sense_len, &cioc->sense_len, sizeof(u32)) ||
- copy_in_user(ioc->frame.raw, cioc->frame.raw, 128) ||
- copy_in_user(&ioc->sge_count, &cioc->sge_count, sizeof(u32)))
- return -EFAULT;
-
- /*
- * The sense_ptr is used in megasas_mgmt_fw_ioctl only when
- * sense_len is not null, so prepare the 64bit value under
- * the same condition.
- */
- if (get_user(local_sense_off, &ioc->sense_off) ||
- get_user(local_sense_len, &ioc->sense_len) ||
- get_user(user_sense_off, &cioc->sense_off))
- return -EFAULT;
-
- if (local_sense_off != user_sense_off)
- return -EINVAL;
-
- if (local_sense_len) {
- void __user **sense_ioc_ptr =
- (void __user **)((u8 *)((unsigned long)&ioc->frame.raw) + local_sense_off);
- compat_uptr_t *sense_cioc_ptr =
- (compat_uptr_t *)(((unsigned long)&cioc->frame.raw) + user_sense_off);
- if (get_user(ptr, sense_cioc_ptr) ||
- put_user(compat_ptr(ptr), sense_ioc_ptr))
- return -EFAULT;
- }
-
- for (i = 0; i < MAX_IOCTL_SGE; i++) {
- if (get_user(ptr, &cioc->sgl[i].iov_base) ||
- put_user(compat_ptr(ptr), &ioc->sgl[i].iov_base) ||
- copy_in_user(&ioc->sgl[i].iov_len,
- &cioc->sgl[i].iov_len, sizeof(compat_size_t)))
- return -EFAULT;
- }
-
- error = megasas_mgmt_ioctl_fw(file, (unsigned long)ioc);
-
- if (copy_in_user(&cioc->frame.hdr.cmd_status,
- &ioc->frame.hdr.cmd_status, sizeof(u8))) {
- printk(KERN_DEBUG "megasas: error copy_in_user cmd_status\n");
- return -EFAULT;
- }
- return error;
-}
-
static long
megasas_mgmt_compat_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
switch (cmd) {
case MEGASAS_IOC_FIRMWARE32:
- return megasas_mgmt_compat_ioctl_fw(file, arg);
+ return megasas_mgmt_ioctl_fw(file, arg);
case MEGASAS_IOC_GET_AEN:
return megasas_mgmt_ioctl_aen(file, arg);
}
--
2.27.0
next prev parent reply other threads:[~2020-09-08 21:38 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-08 21:36 [PATCH 1/3] scsi: aacraid: improve compat_ioctl handlers Arnd Bergmann
2020-09-08 21:36 ` [PATCH 2/3] scsi: megaraid_sas: check user-provided offsets Arnd Bergmann
2020-09-12 7:20 ` Christoph Hellwig
2020-09-12 17:03 ` Arnd Bergmann
2020-12-31 0:15 ` Phil Oester
2021-01-03 16:26 ` Arnd Bergmann
2021-01-03 17:00 ` James Bottomley
2021-01-03 18:49 ` Arnd Bergmann
2021-01-03 20:12 ` James Bottomley
2021-01-04 17:48 ` Phil Oester
2021-01-04 22:24 ` Arnd Bergmann
2020-09-08 21:36 ` Arnd Bergmann [this message]
2020-09-12 7:47 ` [PATCH 3/3] scsi: megaraid_sas: simplify compat_ioctl handling Christoph Hellwig
2020-09-12 12:49 ` Arnd Bergmann
2020-09-13 6:50 ` compat_alloc_user_space removal, was " Christoph Hellwig
2020-09-13 11:46 ` Arnd Bergmann
2020-09-17 14:55 ` Arnd Bergmann
2020-09-17 14:57 ` Christoph Hellwig
2020-09-12 7:09 ` [PATCH 1/3] scsi: aacraid: improve compat_ioctl handlers Christoph Hellwig
2020-09-17 15:02 ` Arnd Bergmann
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=20200908213715.3553098-3-arnd@arndb.de \
--to=arnd@arndb.de \
--cc=anand.lodnoor@broadcom.com \
--cc=chandrakanth.patil@broadcom.com \
--cc=hare@suse.de \
--cc=hch@infradead.org \
--cc=jejb@linux.ibm.com \
--cc=kashyap.desai@broadcom.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=megaraidlinux.pdl@broadcom.com \
--cc=shivasharan.srikanteshwara@broadcom.com \
--cc=sumit.saxena@broadcom.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).