From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Fri, 21 Aug 2015 15:42:21 +0000 Subject: Re: [patch] mptfusion: prevent some memory corruption Message-Id: <20150821154221.GS5610@mwanda> List-Id: References: <20150703085303.GA11901@mwanda> In-Reply-To: <20150703085303.GA11901@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Nagalakshmi Nandigama Cc: Praveen Krishnamoorthy , Sreekanth Reddy , Abhijit Mahajan , MPT-FusionLinux.pdl@avagotech.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Ping? regards, dan carpenter On Fri, Jul 03, 2015 at 11:53:03AM +0300, Dan Carpenter wrote: > These are signed values the come from the user, we put a cap on the > upper bounds but not on the lower bounds. > > We use "karg.dataSgeOffset" to calculate "sz". We verify "sz" and > proceed as if that means that "karg.dataSgeOffset" is correct but this > fails to consider that the "sz" calculations can have integer overflows. > > Signed-off-by: Dan Carpenter > --- > During my QC process, I realized that I sent a similar patch last year, > but never received a response. > > http://permalink.gmane.org/gmane.linux.kernel.janitors/32590 > > Looking at both of them, I guess I prefer today's patch because it is > simpler. > > diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c > index 70bb753..fc73937 100644 > --- a/drivers/message/fusion/mptctl.c > +++ b/drivers/message/fusion/mptctl.c > @@ -1859,6 +1859,15 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr) > } > spin_unlock_irqrestore(&ioc->taskmgmt_lock, flags); > > + /* Basic sanity checks to prevent underflows or integer overflows */ > + if (karg.maxReplyBytes < 0 || > + karg.dataInSize < 0 || > + karg.dataOutSize < 0 || > + karg.dataSgeOffset < 0 || > + karg.maxSenseBytes < 0 || > + karg.dataSgeOffset > ioc->req_sz / 4) > + return -EINVAL; > + > /* Verify that the final request frame will not be too large. > */ > sz = karg.dataSgeOffset * 4;