drivers/scsi/mpt2sas: prevent heap overflows and unchecked reads
PATCH 1/1 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.
--- drivers/scsi/mpt2sas/mpt2sas_ctl.c | 23 +++++++++++++++++++++-- 1 files changed, 21 insertions(+), 2 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_ctl.c +21 -2
@@ -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);
This patch has been tested by Eric Moore and it works fine.
Please consider this patch "Acked-by: Eric Moore <[email protected]>"
Thanks, Kashyap
Quoted 97 lines
-----Original Message-----
From: Dan Rosenberg [mailto:[email protected]]
Sent: Tuesday, April 05, 2011 10:16 PM
To: Moore, Eric; [email protected]; Desai, Kashyap
Cc: [email protected]; [email protected]; linux- [email protected]
Subject: [PATCH]drivers/scsi/mpt2sas: prevent heap overflows and unchecked readsAt 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 <[email protected]>--- drivers/scsi/mpt2sas/mpt2sas_ctl.c | 23 +++++++++++++++++++++-- 1 files changed, 21 insertions(+), 2 deletions(-)drivers/scsi/mpt2sas/mpt2sas_ctl.c +7
-0@@ -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__,drivers/scsi/mpt2sas/mpt2sas_ctl.c +1
-1@@ -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;drivers/scsi/mpt2sas/mpt2sas_ctl.c +2
-0u8 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 "drivers/scsi/mpt2sas/mpt2sas_ctl.c +11
-1"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 "
drivers/scsi/mpt2sas/mpt2sas_ctl.c _ctl_diag_read_buffer(void __user *arg, enum block_state state) · line 2037
Quoted 2 lines
drivers/scsi/mpt2sas/mpt2sas_ctl.c +0
-0"mpt_diag_read_buffer_t data @ %p\n" , ioc->name, __func__, diag_data);
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m����������?�I�