* [PATCH] drivers/scsi/mpt2sas: prevent heap overflows and unchecked reads
@ 2011-04-05 16:45 Dan Rosenberg
2011-04-14 6:08 ` Desai, Kashyap
0 siblings, 1 reply; 2+ messages in thread
From: Dan Rosenberg @ 2011-04-05 16:45 UTC (permalink / raw)
To: eric.moore, james.bottomley, kashyap.desai
Cc: security, linux-scsi, linux-kernel
At two points in handling device ioctls via /dev/mpt2ctl, user-supplied
length values are used to copy data from userspace into heap buffers
without bounds checking, allowing controllable heap corruption and
subsequently privilege escalation.
Additionally, user-supplied values are used to determine the size of a
copy_to_user() as well as the offset into the buffer to be read, with no
bounds checking, allowing users to read arbitrary kernel memory.
Compile-tested only. Thanks to Eric Moore for suggestions on how to fix
this.
Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: stable@kernel.org
---
drivers/scsi/mpt2sas/mpt2sas_ctl.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.c b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
index e92b77a..3834c95 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
@@ -688,6 +688,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
goto out;
}
+ /* Check for overflow and wraparound */
+ if (karg.data_sge_offset * 4 > ioc->request_sz ||
+ karg.data_sge_offset > (UINT_MAX / 4)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
/* copy in request message frame from user */
if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,
@@ -1963,7 +1970,7 @@ _ctl_diag_read_buffer(void __user *arg, enum block_state state)
Mpi2DiagBufferPostReply_t *mpi_reply;
int rc, i;
u8 buffer_type;
- unsigned long timeleft;
+ unsigned long timeleft, request_size, copy_size;
u16 smid;
u16 ioc_status;
u8 issue_reset = 0;
@@ -1999,6 +2006,8 @@ _ctl_diag_read_buffer(void __user *arg, enum block_state state)
return -ENOMEM;
}
+ request_size = ioc->diag_buffer_sz[buffer_type];
+
if ((karg.starting_offset % 4) || (karg.bytes_to_read % 4)) {
printk(MPT2SAS_ERR_FMT "%s: either the starting_offset "
"or bytes_to_read are not 4 byte aligned\n", ioc->name,
@@ -2006,13 +2015,23 @@ _ctl_diag_read_buffer(void __user *arg, enum block_state state)
return -EINVAL;
}
+ if (karg.starting_offset > request_size)
+ return -EINVAL;
+
diag_data = (void *)(request_data + karg.starting_offset);
dctlprintk(ioc, printk(MPT2SAS_INFO_FMT "%s: diag_buffer(%p), "
"offset(%d), sz(%d)\n", ioc->name, __func__,
diag_data, karg.starting_offset, karg.bytes_to_read));
+ /* Truncate data on requests that are too large */
+ if ((diag_data + karg.bytes_to_read < diag_data) ||
+ (diag_data + karg.bytes_to_read > request_data + request_size))
+ copy_size = request_size - karg.starting_offset;
+ else
+ copy_size = karg.bytes_to_read;
+
if (copy_to_user((void __user *)uarg->diagnostic_data,
- diag_data, karg.bytes_to_read)) {
+ diag_data, copy_size)) {
printk(MPT2SAS_ERR_FMT "%s: Unable to write "
"mpt_diag_read_buffer_t data @ %p\n", ioc->name,
__func__, diag_data);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH] drivers/scsi/mpt2sas: prevent heap overflows and unchecked reads
2011-04-05 16:45 [PATCH] drivers/scsi/mpt2sas: prevent heap overflows and unchecked reads Dan Rosenberg
@ 2011-04-14 6:08 ` Desai, Kashyap
0 siblings, 0 replies; 2+ messages in thread
From: Desai, Kashyap @ 2011-04-14 6:08 UTC (permalink / raw)
To: Dan Rosenberg, Moore, Eric, james.bottomley
Cc: security, linux-scsi, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3822 bytes --]
This patch has been tested by Eric Moore and it works fine.
Please consider this patch "Acked-by: Eric Moore <eric.moore@lsi.com>"
Thanks, Kashyap
> -----Original Message-----
> From: Dan Rosenberg [mailto:drosenberg@vsecurity.com]
> Sent: Tuesday, April 05, 2011 10:16 PM
> To: Moore, Eric; james.bottomley@suse.de; Desai, Kashyap
> Cc: security@kernel.org; linux-scsi@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] drivers/scsi/mpt2sas: prevent heap overflows and
> unchecked reads
>
> At two points in handling device ioctls via /dev/mpt2ctl, user-supplied
> length values are used to copy data from userspace into heap buffers
> without bounds checking, allowing controllable heap corruption and
> subsequently privilege escalation.
>
> Additionally, user-supplied values are used to determine the size of a
> copy_to_user() as well as the offset into the buffer to be read, with
> no
> bounds checking, allowing users to read arbitrary kernel memory.
>
> Compile-tested only. Thanks to Eric Moore for suggestions on how to
> fix
> this.
>
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Cc: stable@kernel.org
> ---
> drivers/scsi/mpt2sas/mpt2sas_ctl.c | 23 +++++++++++++++++++++--
> 1 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> index e92b77a..3834c95 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> @@ -688,6 +688,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
> goto out;
> }
>
> + /* Check for overflow and wraparound */
> + if (karg.data_sge_offset * 4 > ioc->request_sz ||
> + karg.data_sge_offset > (UINT_MAX / 4)) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> /* copy in request message frame from user */
> if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
> printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__,
> __LINE__,
> @@ -1963,7 +1970,7 @@ _ctl_diag_read_buffer(void __user *arg, enum
> block_state state)
> Mpi2DiagBufferPostReply_t *mpi_reply;
> int rc, i;
> u8 buffer_type;
> - unsigned long timeleft;
> + unsigned long timeleft, request_size, copy_size;
> u16 smid;
> u16 ioc_status;
> u8 issue_reset = 0;
> @@ -1999,6 +2006,8 @@ _ctl_diag_read_buffer(void __user *arg, enum
> block_state state)
> return -ENOMEM;
> }
>
> + request_size = ioc->diag_buffer_sz[buffer_type];
> +
> if ((karg.starting_offset % 4) || (karg.bytes_to_read % 4)) {
> printk(MPT2SAS_ERR_FMT "%s: either the starting_offset "
> "or bytes_to_read are not 4 byte aligned\n", ioc->name,
> @@ -2006,13 +2015,23 @@ _ctl_diag_read_buffer(void __user *arg, enum
> block_state state)
> return -EINVAL;
> }
>
> + if (karg.starting_offset > request_size)
> + return -EINVAL;
> +
> diag_data = (void *)(request_data + karg.starting_offset);
> dctlprintk(ioc, printk(MPT2SAS_INFO_FMT "%s: diag_buffer(%p), "
> "offset(%d), sz(%d)\n", ioc->name, __func__,
> diag_data, karg.starting_offset, karg.bytes_to_read));
>
> + /* Truncate data on requests that are too large */
> + if ((diag_data + karg.bytes_to_read < diag_data) ||
> + (diag_data + karg.bytes_to_read > request_data +
> request_size))
> + copy_size = request_size - karg.starting_offset;
> + else
> + copy_size = karg.bytes_to_read;
> +
> if (copy_to_user((void __user *)uarg->diagnostic_data,
> - diag_data, karg.bytes_to_read)) {
> + diag_data, copy_size)) {
> printk(MPT2SAS_ERR_FMT "%s: Unable to write "
> "mpt_diag_read_buffer_t data @ %p\n", ioc->name,
> __func__, diag_data);
>
>
>
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-04-14 6:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-05 16:45 [PATCH] drivers/scsi/mpt2sas: prevent heap overflows and unchecked reads Dan Rosenberg
2011-04-14 6:08 ` Desai, Kashyap
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).