Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-13 Thread Petros Koutoupis
On Thu, 2016-05-12 at 09:35 +0300, Dan Carpenter wrote: > On Wed, May 11, 2016 at 08:49:51PM -0500, Petros Koutoupis wrote: > > Sumit, > > > > I will resubmit the patch with all the recommendations. Thank you. In case > > you are interested, I have a crash file showcasing the error. I can always

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-13 Thread Finn Thain
On Fri, 13 May 2016, Sumit Saxena wrote: > > For IOs returned by above error are not actually fired to firmware so > there will be no interrupt handler called for the same. You don't need both possibilities to hold. Either one would do it! -- -- To unsubscribe from this list: send the line

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-13 Thread Sumit Saxena
m; > megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org > Subject: RE: [PATCH] megaraid: add scsi_cmnd NULL check before use > > > On Thu, 12 May 2016, Sumit Saxena wrote: > > > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > > > > > > Also when I'm d

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-13 Thread Finn Thain
On Thu, 12 May 2016, Sumit Saxena wrote: > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > > > > Also when I'm doing static analysis people always tell me that "that > > bug is impossible, trust me." and instead of trusting people I really > > wish they would just show me the

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-12 Thread Sumit Saxena
m; > megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org > Subject: Re: [PATCH] megaraid: add scsi_cmnd NULL check before use > > On Wed, May 11, 2016 at 08:49:51PM -0500, Petros Koutoupis wrote: > > Sumit, > > > > I will resubmit the patch with all the recommendations. T

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-12 Thread Dan Carpenter
On Wed, May 11, 2016 at 08:49:51PM -0500, Petros Koutoupis wrote: > Sumit, > > I will resubmit the patch with all the recommendations. Thank you. In case > you are interested, I have a crash file showcasing the error. I can always > provide this outside of this mailing thread. > Please send it

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-11 Thread Petros Koutoupis
de...@avagotech.com; sumit.sax...@avagotech.com; > > uday.ling...@avagotech.com; megaraidlinux@avagotech.com; linux- > > s...@vger.kernel.org > > Subject: Re: [PATCH] megaraid: add scsi_cmnd NULL check before use > > > > On Mon, 2016-05-09 at 15:18 +0530, Sumit Saxena wrote:

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-11 Thread Sumit Saxena
@avagotech.com; linux- > s...@vger.kernel.org > Subject: Re: [PATCH] megaraid: add scsi_cmnd NULL check before use > > On Mon, 2016-05-09 at 15:18 +0530, Sumit Saxena wrote: > > > > > > -Original Message- > > > From: Dan Carpenter [mailto:dan.carpe

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-09 Thread Petros Koutoupis
.com; > > sumit.sax...@avagotech.com; uday.ling...@avagotech.com; > > megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org > > Subject: Re: [PATCH] megaraid: add scsi_cmnd NULL check before use > > > > Smatch doesn't quite catch it because we check "cmd_fusio

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-09 Thread Dan Carpenter
I'm confused what you are saying. According to Petros, these bugs are causing failures in real life, I was only added to the CC list because Smatch should have warned about them. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a

RE: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-09 Thread Sumit Saxena
otech.com; linux-scsi@vger.kernel.org > Subject: Re: [PATCH] megaraid: add scsi_cmnd NULL check before use > > Smatch doesn't quite catch it because we check "cmd_fusion->scmd" for NULL > then assign "scmd_local = cmd_fusion->scmd;" and dereference scmd_local &g

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-09 Thread Dan Carpenter
I've updated Smatch to catch these and I'm testing it now. We'll see how it goes. If my quick and dirty method doesn't has too many false positives then it's will take some months before I get around to doing it properly... Thanks for notifying me on this. regards, dan carpenter -- To

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-09 Thread Dan Carpenter
Smatch doesn't quite catch it because we check "cmd_fusion->scmd" for NULL then assign "scmd_local = cmd_fusion->scmd;" and dereference scmd_local unconditionally... It does catch part of the bug if you have cross function analysis: drivers/scsi/megaraid/megaraid_sas_fusion.c:2318

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Petros Koutoupis
On Mon, 2016-05-09 at 11:35 +1000, Julian Calaby wrote: > Hi Petros, > > On Mon, May 9, 2016 at 2:34 AM, Petros Koutoupis > wrote: > > On Sun, 2016-05-08 at 22:22 +1000, Finn Thain wrote: > >> On Sun, 8 May 2016, Petros Koutoupis wrote: > >> > >> > > > >> > > That

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Julian Calaby
Hi Petros, On Mon, May 9, 2016 at 2:34 AM, Petros Koutoupis wrote: > On Sun, 2016-05-08 at 22:22 +1000, Finn Thain wrote: >> On Sun, 8 May 2016, Petros Koutoupis wrote: >> >> > > >> > > That contains a tautology. >> > > >> > >> > How so? >> >> if (x) >> /* ...

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Petros Koutoupis
On Sun, 2016-05-08 at 22:22 +1000, Finn Thain wrote: > On Sun, 8 May 2016, Petros Koutoupis wrote: > > > > > > > That contains a tautology. > > > > > > > How so? > > if (x) > /* ... */ > else if (!x && (whatever)) > /* ... */ > > -- Thank you but I know the logic of what I

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Finn Thain
On Sun, 8 May 2016, Petros Koutoupis wrote: > > > > That contains a tautology. > > > > How so? if (x) /* ... */ else if (!x && (whatever)) /* ... */ -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Petros Koutoupis
On Sun, 2016-05-08 at 13:32 +1000, Finn Thain wrote: > On Sat, 7 May 2016, Petros Koutoupis wrote: > > > The current state of the code checks to see if the reference to > > scsi_cmnd is not null, but it never checks to see if it is null and > > always assumes it is valid before its use in below

Re: [PATCH] megaraid: add scsi_cmnd NULL check before use

2016-05-07 Thread Finn Thain
On Sat, 7 May 2016, Petros Koutoupis wrote: > The current state of the code checks to see if the reference to > scsi_cmnd is not null, but it never checks to see if it is null and > always assumes it is valid before its use in below switch statement. > This patch addresses that. > There is