* [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference [not found] <20131025144452.GA28451@ngolde.de> @ 2013-10-29 19:10 ` Dan Carpenter 2013-10-29 19:36 ` James Bottomley ` (2 more replies) 2013-10-29 19:11 ` [patch] [SCSI] aacraid: missing capable() check in compat ioctl Dan Carpenter 2013-10-30 17:13 ` [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() Dan Carpenter 2 siblings, 3 replies; 13+ messages in thread From: Dan Carpenter @ 2013-10-29 19:10 UTC (permalink / raw) To: Adaptec OEM Raid Solutions; +Cc: James E.J. Bottomley, linux-scsi, security If "fibsize" is zero then it leads to a ZERO_SIZE_PTR dereference when we dereference user_srbcmd. Due to a missing capable() check in the compat ioctls then this error can be triggered without CAP_SYS_RAWIO. I have fixed that in a separate patch. Reported-by: Nico Golde <nico@ngolde.de> Reported-by: Fabian Yamaguchi <fabs@goesec.de> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c index d85ac1a..efd0ba3 100644 --- a/drivers/scsi/aacraid/commctrl.c +++ b/drivers/scsi/aacraid/commctrl.c @@ -511,7 +511,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) goto cleanup; } - if (fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr))) { + if (fibsize == 0 || + fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr))) { rcode = -EINVAL; goto cleanup; } ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference 2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter @ 2013-10-29 19:36 ` James Bottomley 2013-10-29 19:59 ` Linus Torvalds 2013-10-30 7:47 ` Dan Carpenter 2 siblings, 0 replies; 13+ messages in thread From: James Bottomley @ 2013-10-29 19:36 UTC (permalink / raw) To: Dan Carpenter; +Cc: Adaptec OEM Raid Solutions, linux-scsi, security On Tue, 2013-10-29 at 22:10 +0300, Dan Carpenter wrote: > If "fibsize" is zero then it leads to a ZERO_SIZE_PTR dereference when > we dereference user_srbcmd. > > Due to a missing capable() check in the compat ioctls then this error > can be triggered without CAP_SYS_RAWIO. I have fixed that in a separate > patch. > > Reported-by: Nico Golde <nico@ngolde.de> > Reported-by: Fabian Yamaguchi <fabs@goesec.de> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c > index d85ac1a..efd0ba3 100644 > --- a/drivers/scsi/aacraid/commctrl.c > +++ b/drivers/scsi/aacraid/commctrl.c > @@ -511,7 +511,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) > goto cleanup; > } > > - if (fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr))) { > + if (fibsize == 0 || > + fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr))) { Not really if you want to catch actual undersize errors. We read out of the allocated data structure after this, so it had better be at least sizeof(*user_srbcmd) rather than just not zero to make sure we aren't reading off the end of the allocated size. James ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference 2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter 2013-10-29 19:36 ` James Bottomley @ 2013-10-29 19:59 ` Linus Torvalds 2013-10-29 20:06 ` Dan Carpenter 2013-10-30 7:47 ` Dan Carpenter 2 siblings, 1 reply; 13+ messages in thread From: Linus Torvalds @ 2013-10-29 19:59 UTC (permalink / raw) To: Dan Carpenter Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley, Linux SCSI List, security@kernel.org On Tue, Oct 29, 2013 at 12:10 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > If "fibsize" is zero then it leads to a ZERO_SIZE_PTR dereference when > we dereference user_srbcmd. Btw, these "ZERO_SIZE_PTR dereference" issues aren't about ZERO_SIZE_PTR, they are about overrunning the allocations. The ZERO_SIZE_PTR pointer is a perfectly valid pointer and can be dereferenced just fine, as long as you stay within the allocation size. Think about it. So I really get the feeling that checking for a zero size is very wrong. If we can access the ZERO_SIZE_PTR, that means that we at some point don't check the size limits, and if that's true for the zero size, I don't see why that wouldn't be true for *other* sizes too.. I didn't check this particular case, and maybe zero really is special. But it's not clear at all why it should be. Can you explain why zero is special here, and why the buffer overrun cannot happen with size 1, for example? Because quite frankly, from what I can tell, testing against zero is absolutely *WRONG*. Any size less than the size of "struct user_aac_srb" would seem to be bogus. This has nothing to do with zero, or with ZERO_SIZE_PTR. Tell me why I'm wrong. Linus ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference 2013-10-29 19:59 ` Linus Torvalds @ 2013-10-29 20:06 ` Dan Carpenter 2013-10-29 20:17 ` Linus Torvalds 0 siblings, 1 reply; 13+ messages in thread From: Dan Carpenter @ 2013-10-29 20:06 UTC (permalink / raw) To: Linus Torvalds Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley, Linux SCSI List, security@kernel.org You and James are right. It should be checking against the sizeof(). I will send a v2 tomorrow. Sorry about that. regards, dan carpenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference 2013-10-29 20:06 ` Dan Carpenter @ 2013-10-29 20:17 ` Linus Torvalds 0 siblings, 0 replies; 13+ messages in thread From: Linus Torvalds @ 2013-10-29 20:17 UTC (permalink / raw) To: Dan Carpenter Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley, Linux SCSI List, security@kernel.org On Tue, Oct 29, 2013 at 1:06 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > You and James are right. It should be checking against the sizeof(). > I will send a v2 tomorrow. Sorry about that. Looking some more at this, I have to say that I absolutely detest those aacraid structures. And I'm not sure that sizeof() is necessarily the right thing for the minimum size. The "struct user_aac_srb" includes a struct user_sgmap sg; which has a count in it. But the actual "struct user_sgmap" structure is defined with a struct user_sgentry sg[1]; in it, so the sizeof() of that structure basically gives the size of an entry that has _one_ sgentry. And it's not entirely clear that you absolutely have to have a minimum of one sgentry. So I could imagine that there would be a zero-entry case that doesn't have any scatter-gather entries at all (ie just the status parts). So the "sizeof()" might actually end up giving a minimum size that is too large *if* it is possible to not have those scatter-gather entries at all? Hmm? Somebody who knows this code, please speak up.. Linus ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference 2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter 2013-10-29 19:36 ` James Bottomley 2013-10-29 19:59 ` Linus Torvalds @ 2013-10-30 7:47 ` Dan Carpenter 2 siblings, 0 replies; 13+ messages in thread From: Dan Carpenter @ 2013-10-30 7:47 UTC (permalink / raw) To: Adaptec OEM Raid Solutions; +Cc: James E.J. Bottomley, linux-scsi, security On Tue, Oct 29, 2013 at 10:10:07PM +0300, Dan Carpenter wrote: > Due to a missing capable() check in the compat ioctls then this error > can be triggered without CAP_SYS_RAWIO. I have fixed that in a separate > patch. Actually, CAP_SYS_RAWIO is checked at the start of the function. However my other patch which adds the check in the compat ioctl should probably still be applied so it matches the regular ioctl. regards, dan carpenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* [patch] [SCSI] aacraid: missing capable() check in compat ioctl [not found] <20131025144452.GA28451@ngolde.de> 2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter @ 2013-10-29 19:11 ` Dan Carpenter 2013-10-30 17:13 ` [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() Dan Carpenter 2 siblings, 0 replies; 13+ messages in thread From: Dan Carpenter @ 2013-10-29 19:11 UTC (permalink / raw) To: Adaptec OEM Raid Solutions Cc: James E.J. Bottomley, linux-scsi, Nico Golde, security In d496f94d22d1 ('[SCSI] aacraid: fix security weakness') we added a check on CAP_SYS_RAWIO to the ioctl. The compat ioctls need the check as well. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 408a42e..f0d432c 100644 --- a/drivers/scsi/aacraid/linit.c +++ b/drivers/scsi/aacraid/linit.c @@ -771,6 +771,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg) { struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata; + if (!capable(CAP_SYS_RAWIO)) + return -EPERM; return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg); } ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() [not found] <20131025144452.GA28451@ngolde.de> 2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter 2013-10-29 19:11 ` [patch] [SCSI] aacraid: missing capable() check in compat ioctl Dan Carpenter @ 2013-10-30 17:13 ` Dan Carpenter 2013-11-21 0:40 ` Kees Cook 2014-01-08 12:27 ` Saxena, Sumit 2 siblings, 2 replies; 13+ messages in thread From: Dan Carpenter @ 2013-10-30 17:13 UTC (permalink / raw) To: Neela Syam Kolli Cc: James E.J. Bottomley, linux-scsi, security, Nico Golde, Fabian Yamaguchi pthru32->dataxferlen comes from the user so we need to check that it's not too large so we don't overflow the buffer. Reported-by: Nico Golde <nico@ngolde.de> Reported-by: Fabian Yamaguchi <fabs@goesec.de> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Please review this carefully because I have not tested it. diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index dfffd0f..a706927 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, mraid_mmadp_t *adp, uioc_t *kioc) pthru32->dataxferaddr = kioc->buf_paddr; if (kioc->data_dir & UIOC_WR) { + if (pthru32->dataxferlen > kioc->xferlen) + return -EINVAL; if (copy_from_user(kioc->buf_vaddr, kioc->user_data, pthru32->dataxferlen)) { return (-EFAULT); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() 2013-10-30 17:13 ` [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() Dan Carpenter @ 2013-11-21 0:40 ` Kees Cook 2014-01-08 12:27 ` Saxena, Sumit 1 sibling, 0 replies; 13+ messages in thread From: Kees Cook @ 2013-11-21 0:40 UTC (permalink / raw) To: Dan Carpenter Cc: Neela Syam Kolli, James E.J. Bottomley, linux-scsi, security@kernel.org, Nico Golde, Fabian Yamaguchi Hi, On Wed, Oct 30, 2013 at 10:13 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > pthru32->dataxferlen comes from the user so we need to check that it's > not too large so we don't overflow the buffer. > > Reported-by: Nico Golde <nico@ngolde.de> > Reported-by: Fabian Yamaguchi <fabs@goesec.de> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> I don't see this in Linus's tree nor linux-next yet. Can someone pick this up? Thanks! -Kees > --- > Please review this carefully because I have not tested it. > > diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c > index dfffd0f..a706927 100644 > --- a/drivers/scsi/megaraid/megaraid_mm.c > +++ b/drivers/scsi/megaraid/megaraid_mm.c > @@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, mraid_mmadp_t *adp, uioc_t *kioc) > > pthru32->dataxferaddr = kioc->buf_paddr; > if (kioc->data_dir & UIOC_WR) { > + if (pthru32->dataxferlen > kioc->xferlen) > + return -EINVAL; > if (copy_from_user(kioc->buf_vaddr, kioc->user_data, > pthru32->dataxferlen)) { > return (-EFAULT); > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() 2013-10-30 17:13 ` [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() Dan Carpenter 2013-11-21 0:40 ` Kees Cook @ 2014-01-08 12:27 ` Saxena, Sumit 2014-01-09 18:34 ` Kees Cook 1 sibling, 1 reply; 13+ messages in thread From: Saxena, Sumit @ 2014-01-08 12:27 UTC (permalink / raw) To: Dan Carpenter, DL-MegaRAID Linux Cc: James E.J. Bottomley, linux-scsi@vger.kernel.org, security@kernel.org, Nico Golde, Fabian Yamaguchi >-----Original Message----- >From: Dan Carpenter [mailto:dan.carpenter@oracle.com] >Sent: Wednesday, October 30, 2013 10:44 PM >To: DL-MegaRAID Linux >Cc: James E.J. Bottomley; linux-scsi@vger.kernel.org; security@kernel.org; >Nico Golde; Fabian Yamaguchi >Subject: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() > >pthru32->dataxferlen comes from the user so we need to check that it's >not too large so we don't overflow the buffer. > >Reported-by: Nico Golde <nico@ngolde.de> >Reported-by: Fabian Yamaguchi <fabs@goesec.de> >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >--- >Please review this carefully because I have not tested it. > >diff --git a/drivers/scsi/megaraid/megaraid_mm.c >b/drivers/scsi/megaraid/megaraid_mm.c >index dfffd0f..a706927 100644 >--- a/drivers/scsi/megaraid/megaraid_mm.c >+++ b/drivers/scsi/megaraid/megaraid_mm.c >@@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, >mraid_mmadp_t *adp, uioc_t *kioc) > > pthru32->dataxferaddr = kioc->buf_paddr; > if (kioc->data_dir & UIOC_WR) { >+ if (pthru32->dataxferlen > kioc->xferlen) >+ return -EINVAL; > if (copy_from_user(kioc->buf_vaddr, kioc->user_data, > pthru32->dataxferlen)) { > return (-EFAULT); Acked-by: Sumit Saxena <sumit.saxena@lsi.com> Sumit ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() 2014-01-08 12:27 ` Saxena, Sumit @ 2014-01-09 18:34 ` Kees Cook 2014-01-09 19:53 ` Saxena, Sumit 0 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2014-01-09 18:34 UTC (permalink / raw) To: Saxena, Sumit Cc: Dan Carpenter, DL-MegaRAID Linux, James E.J. Bottomley, linux-scsi@vger.kernel.org, security@kernel.org, Nico Golde, Fabian Yamaguchi On Wed, Jan 8, 2014 at 4:27 AM, Saxena, Sumit <Sumit.Saxena@lsi.com> wrote: > > >>-----Original Message----- >>From: Dan Carpenter [mailto:dan.carpenter@oracle.com] >>Sent: Wednesday, October 30, 2013 10:44 PM >>To: DL-MegaRAID Linux >>Cc: James E.J. Bottomley; linux-scsi@vger.kernel.org; security@kernel.org; >>Nico Golde; Fabian Yamaguchi >>Subject: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() >> >>pthru32->dataxferlen comes from the user so we need to check that it's >>not too large so we don't overflow the buffer. >> >>Reported-by: Nico Golde <nico@ngolde.de> >>Reported-by: Fabian Yamaguchi <fabs@goesec.de> >>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>--- >>Please review this carefully because I have not tested it. >> >>diff --git a/drivers/scsi/megaraid/megaraid_mm.c >>b/drivers/scsi/megaraid/megaraid_mm.c >>index dfffd0f..a706927 100644 >>--- a/drivers/scsi/megaraid/megaraid_mm.c >>+++ b/drivers/scsi/megaraid/megaraid_mm.c >>@@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, >>mraid_mmadp_t *adp, uioc_t *kioc) >> >> pthru32->dataxferaddr = kioc->buf_paddr; >> if (kioc->data_dir & UIOC_WR) { >>+ if (pthru32->dataxferlen > kioc->xferlen) >>+ return -EINVAL; >> if (copy_from_user(kioc->buf_vaddr, kioc->user_data, >> pthru32->dataxferlen)) { >> return (-EFAULT); > > Acked-by: Sumit Saxena <sumit.saxena@lsi.com> > > Sumit > Thanks for the Ack. Who normally picks patches for this area? -Kees -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() 2014-01-09 18:34 ` Kees Cook @ 2014-01-09 19:53 ` Saxena, Sumit 2014-01-09 20:03 ` Kees Cook 0 siblings, 1 reply; 13+ messages in thread From: Saxena, Sumit @ 2014-01-09 19:53 UTC (permalink / raw) To: Kees Cook Cc: Dan Carpenter, DL-MegaRAID Linux, James E.J. Bottomley, linux-scsi@vger.kernel.org, security@kernel.org, Nico Golde, Fabian Yamaguchi >-----Original Message----- >From: Kees Cook [mailto:keescook@google.com] >Sent: Friday, January 10, 2014 12:05 AM >To: Saxena, Sumit >Cc: Dan Carpenter; DL-MegaRAID Linux; James E.J. Bottomley; linux- >scsi@vger.kernel.org; security@kernel.org; Nico Golde; Fabian Yamaguchi >Subject: Re: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() > >On Wed, Jan 8, 2014 at 4:27 AM, Saxena, Sumit <Sumit.Saxena@lsi.com> >wrote: >> >> >>>-----Original Message----- >>>From: Dan Carpenter [mailto:dan.carpenter@oracle.com] >>>Sent: Wednesday, October 30, 2013 10:44 PM >>>To: DL-MegaRAID Linux >>>Cc: James E.J. Bottomley; linux-scsi@vger.kernel.org; >>>security@kernel.org; Nico Golde; Fabian Yamaguchi >>>Subject: [patch] [SCSI] megaraid: missing bounds check in >>>mimd_to_kioc() >>> >>>pthru32->dataxferlen comes from the user so we need to check that it's >>>not too large so we don't overflow the buffer. >>> >>>Reported-by: Nico Golde <nico@ngolde.de> >>>Reported-by: Fabian Yamaguchi <fabs@goesec.de> >>>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>>--- >>>Please review this carefully because I have not tested it. >>> >>>diff --git a/drivers/scsi/megaraid/megaraid_mm.c >>>b/drivers/scsi/megaraid/megaraid_mm.c >>>index dfffd0f..a706927 100644 >>>--- a/drivers/scsi/megaraid/megaraid_mm.c >>>+++ b/drivers/scsi/megaraid/megaraid_mm.c >>>@@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, >mraid_mmadp_t >>>*adp, uioc_t *kioc) >>> >>> pthru32->dataxferaddr = kioc->buf_paddr; >>> if (kioc->data_dir & UIOC_WR) { >>>+ if (pthru32->dataxferlen > kioc->xferlen) >>>+ return -EINVAL; >>> if (copy_from_user(kioc->buf_vaddr, kioc->user_data, >>> pthru32->dataxferlen)) { >>> return (-EFAULT); >> >> Acked-by: Sumit Saxena <sumit.saxena@lsi.com> >> >> Sumit >> > >Thanks for the Ack. Who normally picks patches for this area? > >-Kees > James Bottomley(Linux SCSI subsystem maintainer) should pick this patch. Sumit >-- >Kees Cook >Chrome OS Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() 2014-01-09 19:53 ` Saxena, Sumit @ 2014-01-09 20:03 ` Kees Cook 0 siblings, 0 replies; 13+ messages in thread From: Kees Cook @ 2014-01-09 20:03 UTC (permalink / raw) To: James E.J. Bottomley Cc: Saxena, Sumit, Dan Carpenter, DL-MegaRAID Linux, linux-scsi@vger.kernel.org, security@kernel.org, Nico Golde, Fabian Yamaguchi On Thu, Jan 9, 2014 at 11:53 AM, Saxena, Sumit <Sumit.Saxena@lsi.com> wrote: > > >>-----Original Message----- >>From: Kees Cook [mailto:keescook@google.com] >>Sent: Friday, January 10, 2014 12:05 AM >>To: Saxena, Sumit >>Cc: Dan Carpenter; DL-MegaRAID Linux; James E.J. Bottomley; linux- >>scsi@vger.kernel.org; security@kernel.org; Nico Golde; Fabian Yamaguchi >>Subject: Re: [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() >> >>On Wed, Jan 8, 2014 at 4:27 AM, Saxena, Sumit <Sumit.Saxena@lsi.com> >>wrote: >>> >>> >>>>-----Original Message----- >>>>From: Dan Carpenter [mailto:dan.carpenter@oracle.com] >>>>Sent: Wednesday, October 30, 2013 10:44 PM >>>>To: DL-MegaRAID Linux >>>>Cc: James E.J. Bottomley; linux-scsi@vger.kernel.org; >>>>security@kernel.org; Nico Golde; Fabian Yamaguchi >>>>Subject: [patch] [SCSI] megaraid: missing bounds check in >>>>mimd_to_kioc() >>>> >>>>pthru32->dataxferlen comes from the user so we need to check that it's >>>>not too large so we don't overflow the buffer. >>>> >>>>Reported-by: Nico Golde <nico@ngolde.de> >>>>Reported-by: Fabian Yamaguchi <fabs@goesec.de> >>>>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>>>--- >>>>Please review this carefully because I have not tested it. >>>> >>>>diff --git a/drivers/scsi/megaraid/megaraid_mm.c >>>>b/drivers/scsi/megaraid/megaraid_mm.c >>>>index dfffd0f..a706927 100644 >>>>--- a/drivers/scsi/megaraid/megaraid_mm.c >>>>+++ b/drivers/scsi/megaraid/megaraid_mm.c >>>>@@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, >>mraid_mmadp_t >>>>*adp, uioc_t *kioc) >>>> >>>> pthru32->dataxferaddr = kioc->buf_paddr; >>>> if (kioc->data_dir & UIOC_WR) { >>>>+ if (pthru32->dataxferlen > kioc->xferlen) >>>>+ return -EINVAL; >>>> if (copy_from_user(kioc->buf_vaddr, kioc->user_data, >>>> pthru32->dataxferlen)) { >>>> return (-EFAULT); >>> >>> Acked-by: Sumit Saxena <sumit.saxena@lsi.com> >>> >>> Sumit >>> >> >>Thanks for the Ack. Who normally picks patches for this area? >> >>-Kees >> > James Bottomley(Linux SCSI subsystem maintainer) should pick this patch. Okay, thanks. James, can you pick up this patch as well? https://lkml.org/lkml/2013/12/18/477 Thanks, -Kees > > Sumit >>-- >>Kees Cook >>Chrome OS Security > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-01-09 20:03 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20131025144452.GA28451@ngolde.de>
2013-10-29 19:10 ` [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference Dan Carpenter
2013-10-29 19:36 ` James Bottomley
2013-10-29 19:59 ` Linus Torvalds
2013-10-29 20:06 ` Dan Carpenter
2013-10-29 20:17 ` Linus Torvalds
2013-10-30 7:47 ` Dan Carpenter
2013-10-29 19:11 ` [patch] [SCSI] aacraid: missing capable() check in compat ioctl Dan Carpenter
2013-10-30 17:13 ` [patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc() Dan Carpenter
2013-11-21 0:40 ` Kees Cook
2014-01-08 12:27 ` Saxena, Sumit
2014-01-09 18:34 ` Kees Cook
2014-01-09 19:53 ` Saxena, Sumit
2014-01-09 20:03 ` Kees Cook
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).