Patchwork [CVE-2011-1494,Maverick] mpt2sas: prevent heap overflows and unchecked reads, CVE-2011-1494

login
register
mail settings
Submitter Herton Ronaldo Krzesinski
Date May 23, 2011, 8:57 p.m.
Message ID <1306184271-5388-1-git-send-email-herton.krzesinski@canonical.com>
Download mbox | patch
Permalink /patch/97059/
State New
Headers show

Comments

Herton Ronaldo Krzesinski - May 23, 2011, 8:57 p.m.
From: Dan Rosenberg <drosenberg@vsecurity.com>

CVE-2011-1494

BugLink: http://bugs.launchpad.net/bugs/787145

Released until now with stable versions 2.6.32.40, 2.6.33.13, 2.6.38.6

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.

Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: stable@kernel.org
Acked-by: Eric Moore <eric.moore@lsi.com>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
(backported from commit a1f74ae82d133ebb2aabb19d181944b4e83e9960 upstream)
Signed-off-by: Herton Krzesinski <herton.krzesinski@canonical.com>
---
 drivers/scsi/mpt2sas/mpt2sas_ctl.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)
John Johansen - May 23, 2011, 11:32 p.m.
On 05/23/2011 01:57 PM, Herton Ronaldo Krzesinski wrote:
> From: Dan Rosenberg <drosenberg@vsecurity.com>
> 
> CVE-2011-1494
> 
> BugLink: http://bugs.launchpad.net/bugs/787145
> 
> Released until now with stable versions 2.6.32.40, 2.6.33.13, 2.6.38.6
> 
> 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.
> 
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Cc: stable@kernel.org
> Acked-by: Eric Moore <eric.moore@lsi.com>
> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
> (backported from commit a1f74ae82d133ebb2aabb19d181944b4e83e9960 upstream)
> Signed-off-by: Herton Krzesinski <herton.krzesinski@canonical.com>

Acked-by: John Johansen <john.johansen@canonical.com>

> ---
>  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 d88e975..9e689c8 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
>  	data_out_sz = karg.data_out_size;
>  	data_in_sz = karg.data_in_size;
>  
> +	/* 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__,
> @@ -1883,7 +1890,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;
> @@ -1919,6 +1926,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,
> @@ -1926,13 +1935,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_DEBUG_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);
Stefan Bader - May 24, 2011, 7:28 a.m.
On 23.05.2011 22:57, Herton Ronaldo Krzesinski wrote:
> From: Dan Rosenberg<drosenberg@vsecurity.com>
>
> CVE-2011-1494
>
> BugLink: http://bugs.launchpad.net/bugs/787145
>
> Released until now with stable versions 2.6.32.40, 2.6.33.13, 2.6.38.6
>
> 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.
>
> Signed-off-by: Dan Rosenberg<drosenberg@vsecurity.com>
> Cc: stable@kernel.org
> Acked-by: Eric Moore<eric.moore@lsi.com>
> Signed-off-by: James Bottomley<James.Bottomley@suse.de>
> (backported from commit a1f74ae82d133ebb2aabb19d181944b4e83e9960 upstream)
> Signed-off-by: Herton Krzesinski<herton.krzesinski@canonical.com>
> ---
>   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 d88e975..9e689c8 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
>   	data_out_sz = karg.data_out_size;
>   	data_in_sz = karg.data_in_size;
>
> +	/* 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__,
> @@ -1883,7 +1890,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;
> @@ -1919,6 +1926,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,
> @@ -1926,13 +1935,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_DEBUG_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);
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Tetsuo Handa - May 24, 2011, 8:23 a.m.
Stefan Bader wrote:
> > --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> > +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> > @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
> >   	data_out_sz = karg.data_out_size;
> >   	data_in_sz = karg.data_in_size;
> >
> > +	/* 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__,

karg.data_sge_offset is assigned from user-supplied memory.
I don't know whether ioc->request_sz can be set to 0.

But if ioc->request_sz can be set to 0,

683         mpi_request = kzalloc(ioc->request_sz, GFP_KERNEL);

mpi_request is set to ZERO_SIZE_PTR

684         if (!mpi_request) {
685                 printk(MPT2SAS_ERR_FMT "%s: failed obtaining a memory for "
686                     "mpi_request\n", ioc->name, __func__);
687                 ret = -ENOMEM;
688                 goto out;
689         }
690 

and reaches here. If karg.data_sge_offset == 0,

691         /* Check for overflow and wraparound */
692         if (karg.data_sge_offset * 4 > ioc->request_sz ||
693             karg.data_sge_offset > (UINT_MAX / 4)) {
694                 ret = -EINVAL;
695                 goto out;
696         }
697 

reaches here.

copy_from_user() copies no data, but

698         /* copy in request message frame from user */
699         if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
700                 printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,
701                     __func__);
702                 ret = -EFAULT;
703                 goto out;
704         }
705 

accessing ZERO_SIZE_PTR->Function is an oops.

706         if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) {

Many functions are passing a variable to (e.g.) kmalloc() with an
assumption that the value of that variable is never 0.

Although this was once NACKed at http://lkml.org/lkml/2010/2/26/30 , don't we
want memory allocator functions that return NULL if requested size is 0?


At line 706, mpi_request->Function is always 0 if karg.data_sge_offset == 0 and
ioc->request_sz != 0 ... Maybe we need karg.data_sge_offset != 0 check.
John Johansen - May 24, 2011, 9:35 a.m.
On 05/24/2011 01:23 AM, Tetsuo Handa wrote:
> Stefan Bader wrote:
>>> --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
>>> @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
>>>   	data_out_sz = karg.data_out_size;
>>>   	data_in_sz = karg.data_in_size;
>>>
>>> +	/* 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__,
> 
> karg.data_sge_offset is assigned from user-supplied memory.
> I don't know whether ioc->request_sz can be set to 0.
> 
Right, I did poke through at this and didn't come up with a satisfactory answer.
I need to spend more time with the code to figure that case out.

> But if ioc->request_sz can be set to 0,
> 
> 683         mpi_request = kzalloc(ioc->request_sz, GFP_KERNEL);
> 
> mpi_request is set to ZERO_SIZE_PTR
> 
> 684         if (!mpi_request) {
> 685                 printk(MPT2SAS_ERR_FMT "%s: failed obtaining a memory for "
> 686                     "mpi_request\n", ioc->name, __func__);
> 687                 ret = -ENOMEM;
> 688                 goto out;
> 689         }
> 690 
> 
> and reaches here. If karg.data_sge_offset == 0,
> 
> 691         /* Check for overflow and wraparound */
> 692         if (karg.data_sge_offset * 4 > ioc->request_sz ||
> 693             karg.data_sge_offset > (UINT_MAX / 4)) {
> 694                 ret = -EINVAL;
> 695                 goto out;
> 696         }
> 697 
> 
> reaches here.
> 
> copy_from_user() copies no data, but
> 
> 698         /* copy in request message frame from user */
> 699         if (copy_from_user(mpi_request, mf, karg.data_sge_offset*4)) {
> 700                 printk(KERN_ERR "failure at %s:%d/%s()!\n", __FILE__, __LINE__,
> 701                     __func__);
> 702                 ret = -EFAULT;
> 703                 goto out;
> 704         }
> 705 
> 
> accessing ZERO_SIZE_PTR->Function is an oops.
> 
yep

> 706         if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) {
> 
> Many functions are passing a variable to (e.g.) kmalloc() with an
> assumption that the value of that variable is never 0.
> 
> Although this was once NACKed at http://lkml.org/lkml/2010/2/26/30 , don't we
> want memory allocator functions that return NULL if requested size is 0?
> 
while I agree with you, and would actually prefer to see kmalloc style functions
that can return ZERO_SIZE_PTR as wrappers around the kernels kmalloc, and code
that actually needs that feature call them explicitly.  I think we are stuck with
the current behavior.

> 
> At line 706, mpi_request->Function is always 0 if karg.data_sge_offset == 0 and
> ioc->request_sz != 0 ... Maybe we need karg.data_sge_offset != 0 check.
> 

That is a check that I am not comfortable making with out having better
understanding of what the code is doing.  From my cursory reading of the code
I would say that karg.data_sge_offset == 0 might be a valid value.  Its something
that really needs to be taken up with the maintainer.

From an SRU pov, the patch is a clean back port of an upstream patch and it does
address the bug, so it is worth taking in its current form.

thanks Tetsuo for spending time doing your review
Steve Conklin - May 24, 2011, 9:13 p.m.
Applied

On Mon, 2011-05-23 at 17:57 -0300, Herton Ronaldo Krzesinski wrote:
> From: Dan Rosenberg <drosenberg@vsecurity.com>
> 
> CVE-2011-1494
> 
> BugLink: http://bugs.launchpad.net/bugs/787145
> 
> Released until now with stable versions 2.6.32.40, 2.6.33.13, 2.6.38.6
> 
> 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.
> 
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Cc: stable@kernel.org
> Acked-by: Eric Moore <eric.moore@lsi.com>
> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
> (backported from commit a1f74ae82d133ebb2aabb19d181944b4e83e9960 upstream)
> Signed-off-by: Herton Krzesinski <herton.krzesinski@canonical.com>
> ---
>  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 d88e975..9e689c8 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
> @@ -637,6 +637,13 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
>  	data_out_sz = karg.data_out_size;
>  	data_in_sz = karg.data_in_size;
>  
> +	/* 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__,
> @@ -1883,7 +1890,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;
> @@ -1919,6 +1926,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,
> @@ -1926,13 +1935,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_DEBUG_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);
> -- 
> 1.7.4.1
> 
>

Patch

diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.c b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
index d88e975..9e689c8 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c
@@ -637,6 +637,13 @@  _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc,
 	data_out_sz = karg.data_out_size;
 	data_in_sz = karg.data_in_size;
 
+	/* 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__,
@@ -1883,7 +1890,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;
@@ -1919,6 +1926,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,
@@ -1926,13 +1935,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_DEBUG_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);