public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
@ 2005-11-23 22:34 Adrian Bunk
  0 siblings, 0 replies; 7+ messages in thread
From: Adrian Bunk @ 2005-11-23 22:34 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Marcelo Tosatti
  Cc: markus.lidel, James.Bottomley, linux-scsi, linux-kernel,
	Mark Salyzyn

The Coverity checker spotted this obvious NULL pointer dereference.


Signed-off-by: Adrian Bunk <bunk@stusta.de>
Acked-by: Mark Salyzyn <mark_salyzyn@adaptec.com>

---

This patch was already sent on:
- 21 Nov 2005

 drivers/scsi/dpt_i2o.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c.old	2005-11-20 22:13:37.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c	2005-11-20 22:16:57.000000000 +0100
@@ -816,7 +816,7 @@
 static void adpt_i2o_sys_shutdown(void)
 {
 	adpt_hba *pHba, *pNext;
-	struct adpt_i2o_post_wait_data *p1, *p2;
+	struct adpt_i2o_post_wait_data *p1, *old;
 
 	 printk(KERN_INFO"Shutting down Adaptec I2O controllers.\n");
 	 printk(KERN_INFO"   This could take a few minutes if there are many devices attached\n");
@@ -830,13 +830,14 @@
 	}
 
 	/* Remove any timedout entries from the wait queue.  */
-	p2 = NULL;
 //	spin_lock_irqsave(&adpt_post_wait_lock, flags);
 	/* Nothing should be outstanding at this point so just
 	 * free them 
 	 */
-	for(p1 = adpt_post_wait_queue; p1; p2 = p1, p1 = p2->next) {
-		kfree(p1);
+	for(p1 = adpt_post_wait_queue; p1;) {
+		old = p1;
+		p1 = p1->next;
+		kfree(old);
 	}
 //	spin_unlock_irqrestore(&adpt_post_wait_lock, flags);
 	adpt_post_wait_queue = NULL;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
@ 2005-11-26 23:36 Adrian Bunk
  2005-11-27 12:47 ` Marcelo Tosatti
  2005-11-28 18:37 ` James Bottomley
  0 siblings, 2 replies; 7+ messages in thread
From: Adrian Bunk @ 2005-11-26 23:36 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Marcelo Tosatti
  Cc: markus.lidel, James.Bottomley, linux-scsi, linux-kernel,
	Mark Salyzyn

The Coverity checker spotted this obvious NULL pointer dereference.


Signed-off-by: Adrian Bunk <bunk@stusta.de>
Acked-by: Mark Salyzyn <mark_salyzyn@adaptec.com>

---

This patch was already sent on:
- 23 Nov 2005
- 21 Nov 2005

 drivers/scsi/dpt_i2o.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c.old	2005-11-20 22:13:37.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c	2005-11-20 22:16:57.000000000 +0100
@@ -816,7 +816,7 @@
 static void adpt_i2o_sys_shutdown(void)
 {
 	adpt_hba *pHba, *pNext;
-	struct adpt_i2o_post_wait_data *p1, *p2;
+	struct adpt_i2o_post_wait_data *p1, *old;
 
 	 printk(KERN_INFO"Shutting down Adaptec I2O controllers.\n");
 	 printk(KERN_INFO"   This could take a few minutes if there are many devices attached\n");
@@ -830,13 +830,14 @@
 	}
 
 	/* Remove any timedout entries from the wait queue.  */
-	p2 = NULL;
 //	spin_lock_irqsave(&adpt_post_wait_lock, flags);
 	/* Nothing should be outstanding at this point so just
 	 * free them 
 	 */
-	for(p1 = adpt_post_wait_queue; p1; p2 = p1, p1 = p2->next) {
-		kfree(p1);
+	for(p1 = adpt_post_wait_queue; p1;) {
+		old = p1;
+		p1 = p1->next;
+		kfree(old);
 	}
 //	spin_unlock_irqrestore(&adpt_post_wait_lock, flags);
 	adpt_post_wait_queue = NULL;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
  2005-11-26 23:36 [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference Adrian Bunk
@ 2005-11-27 12:47 ` Marcelo Tosatti
  2005-11-27 18:52   ` Adrian Bunk
  2005-11-28 18:37 ` James Bottomley
  1 sibling, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2005-11-27 12:47 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Linus Torvalds, Andrew Morton, markus.lidel, James.Bottomley,
	linux-scsi, linux-kernel, Mark Salyzyn

On Sun, Nov 27, 2005 at 12:36:37AM +0100, Adrian Bunk wrote:
> The Coverity checker spotted this obvious NULL pointer dereference.

Hi Adrian,

Could you explain why you remove the adpt_post_wait_lock acquision? 

And if it does not belong there, why don't you remove it instead of
commeting out?

> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> Acked-by: Mark Salyzyn <mark_salyzyn@adaptec.com>
> 
> ---
> 
> This patch was already sent on:
> - 23 Nov 2005
> - 21 Nov 2005
> 
>  drivers/scsi/dpt_i2o.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> --- linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c.old	2005-11-20 22:13:37.000000000 +0100
> +++ linux-2.6.15-rc1-mm2-full/drivers/scsi/dpt_i2o.c	2005-11-20 22:16:57.000000000 +0100
> @@ -816,7 +816,7 @@
>  static void adpt_i2o_sys_shutdown(void)
>  {
>  	adpt_hba *pHba, *pNext;
> -	struct adpt_i2o_post_wait_data *p1, *p2;
> +	struct adpt_i2o_post_wait_data *p1, *old;
>  
>  	 printk(KERN_INFO"Shutting down Adaptec I2O controllers.\n");
>  	 printk(KERN_INFO"   This could take a few minutes if there are many devices attached\n");
> @@ -830,13 +830,14 @@
>  	}
>  
>  	/* Remove any timedout entries from the wait queue.  */
> -	p2 = NULL;
>  //	spin_lock_irqsave(&adpt_post_wait_lock, flags);
>  	/* Nothing should be outstanding at this point so just
>  	 * free them 
>  	 */
> -	for(p1 = adpt_post_wait_queue; p1; p2 = p1, p1 = p2->next) {
> -		kfree(p1);
> +	for(p1 = adpt_post_wait_queue; p1;) {
> +		old = p1;
> +		p1 = p1->next;
> +		kfree(old);
>  	}
>  //	spin_unlock_irqrestore(&adpt_post_wait_lock, flags);
>  	adpt_post_wait_queue = NULL;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
  2005-11-27 18:52   ` Adrian Bunk
@ 2005-11-27 13:28     ` Marcelo Tosatti
  0 siblings, 0 replies; 7+ messages in thread
From: Marcelo Tosatti @ 2005-11-27 13:28 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Linus Torvalds, Andrew Morton, markus.lidel, James.Bottomley,
	linux-scsi, linux-kernel, Mark Salyzyn

On Sun, Nov 27, 2005 at 07:52:52PM +0100, Adrian Bunk wrote:
> On Sun, Nov 27, 2005 at 10:47:38AM -0200, Marcelo Tosatti wrote:
> > On Sun, Nov 27, 2005 at 12:36:37AM +0100, Adrian Bunk wrote:
> > > The Coverity checker spotted this obvious NULL pointer dereference.
> > 
> > Hi Adrian,
> 
> Hi Marcelo,
> 
> > Could you explain why you remove the adpt_post_wait_lock acquision? 
> > 
> > And if it does not belong there, why don't you remove it instead of
> > commeting out?
> >...
> 
> my patch does remove the following not required line:
> 
> > > -	p2 = NULL;
> 
> It does not touch the following line that was already commented out:
> 
> > >  //	spin_lock_irqsave(&adpt_post_wait_lock, flags);
> >...

Doh. I should _read_ the patch. 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
  2005-11-27 12:47 ` Marcelo Tosatti
@ 2005-11-27 18:52   ` Adrian Bunk
  2005-11-27 13:28     ` Marcelo Tosatti
  0 siblings, 1 reply; 7+ messages in thread
From: Adrian Bunk @ 2005-11-27 18:52 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Linus Torvalds, Andrew Morton, markus.lidel, James.Bottomley,
	linux-scsi, linux-kernel, Mark Salyzyn

On Sun, Nov 27, 2005 at 10:47:38AM -0200, Marcelo Tosatti wrote:
> On Sun, Nov 27, 2005 at 12:36:37AM +0100, Adrian Bunk wrote:
> > The Coverity checker spotted this obvious NULL pointer dereference.
> 
> Hi Adrian,

Hi Marcelo,

> Could you explain why you remove the adpt_post_wait_lock acquision? 
> 
> And if it does not belong there, why don't you remove it instead of
> commeting out?
>...

my patch does remove the following not required line:

> > -	p2 = NULL;

It does not touch the following line that was already commented out:

> >  //	spin_lock_irqsave(&adpt_post_wait_lock, flags);
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
  2005-11-26 23:36 [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference Adrian Bunk
  2005-11-27 12:47 ` Marcelo Tosatti
@ 2005-11-28 18:37 ` James Bottomley
  2005-11-28 21:51   ` Adrian Bunk
  1 sibling, 1 reply; 7+ messages in thread
From: James Bottomley @ 2005-11-28 18:37 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Linus Torvalds, Andrew Morton, Marcelo Tosatti, markus.lidel,
	linux-scsi, linux-kernel, Mark Salyzyn

On Sun, 2005-11-27 at 00:36 +0100, Adrian Bunk wrote:
> The Coverity checker spotted this obvious NULL pointer dereference.

It's a bit late for this one, since Linus already put it in, but for
future reference, could you please try to use proper descriptions.  This
isn't an "obvious NULL pointer dereference", it's actually a use after
free of a data structure.

Thanks,

James



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference
  2005-11-28 18:37 ` James Bottomley
@ 2005-11-28 21:51   ` Adrian Bunk
  0 siblings, 0 replies; 7+ messages in thread
From: Adrian Bunk @ 2005-11-28 21:51 UTC (permalink / raw)
  To: James Bottomley
  Cc: Linus Torvalds, Andrew Morton, Marcelo Tosatti, markus.lidel,
	linux-scsi, linux-kernel, Mark Salyzyn

On Mon, Nov 28, 2005 at 12:37:11PM -0600, James Bottomley wrote:
> On Sun, 2005-11-27 at 00:36 +0100, Adrian Bunk wrote:
> > The Coverity checker spotted this obvious NULL pointer dereference.
> 
> It's a bit late for this one, since Linus already put it in, but for
> future reference, could you please try to use proper descriptions.  This
> isn't an "obvious NULL pointer dereference", it's actually a use after
> free of a data structure.

OK sorry, I'll look better at the descriptions for future patches.

> Thanks,
> 
> James

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2005-11-28 21:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-26 23:36 [patch] drivers/scsi/dpt_i2o.c: fix a NULL pointer dereference Adrian Bunk
2005-11-27 12:47 ` Marcelo Tosatti
2005-11-27 18:52   ` Adrian Bunk
2005-11-27 13:28     ` Marcelo Tosatti
2005-11-28 18:37 ` James Bottomley
2005-11-28 21:51   ` Adrian Bunk
  -- strict thread matches above, loose matches on Subject: below --
2005-11-23 22:34 Adrian Bunk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox