public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] Fix bug #13853: dereference pointer 'dev' before null check
@ 2011-03-03  2:20 Jinqiu
  2011-03-03  2:50 ` Li Zefan
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jinqiu @ 2011-03-03  2:20 UTC (permalink / raw)
  To: linux-kernel

This patch fixes bug #13853 .This still exists in latest version.
'dev' is dereferenced before testing for NULL.

Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
--- a/drivers/net/ariadne.c
+++ b/drivers/net/ariadne.c
@@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
static irqreturn_t ariadne_interrupt(int irq, void *data)
{
	struct net_device *dev = (struct net_device *) data;
- 	volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
+ 	volatile struct Am79C960 *lance;
	struct ariadne_private *priv;
	int csr0, boguscnt;
	int handled = 0;
@@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
		printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
		return IRQ_NONE;
	}
+ 	lance = (struct Am79C960 *)dev->base_addr;

	lance->RAP = CSR0; 					/* PCnet-ISA Controller Status */





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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  2:20 [patch] Fix bug #13853: dereference pointer 'dev' before null check Jinqiu
@ 2011-03-03  2:50 ` Li Zefan
  2011-03-03  8:11   ` Wolfram Sang
  2011-03-03 16:42   ` Randy Dunlap
  2011-03-03 19:23 ` Randy Dunlap
  2011-03-03 19:29 ` Randy Dunlap
  2 siblings, 2 replies; 9+ messages in thread
From: Li Zefan @ 2011-03-03  2:50 UTC (permalink / raw)
  To: Jinqiu; +Cc: linux-kernel

So you've been told to read Documentation/SubmittingPatches
and Documentation/email-clients.txt, but there are still quite
a few problems in your patch..

Don't ever mention "bug #13853", because this number makes no sense
for everyone except you. A proper subject may be:

[PATCH] ariadne: fix possible null dereference.

And please send your patches to proper maintainers and mailing list,
David Miller and netdev@vger.kernel.org in particular.

Jinqiu wrote:
> This patch fixes bug #13853 .This still exists in latest version.

Of course it exists, otherwise your patch won't be applied, so just
remove this line from your changelog.

> 'dev' is dereferenced before testing for NULL.
> 
> Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
> --- a/drivers/net/ariadne.c
> +++ b/drivers/net/ariadne.c
> @@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
> static irqreturn_t ariadne_interrupt(int irq, void *data)
> {
>     struct net_device *dev = (struct net_device *) data;
> -     volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
> +     volatile struct Am79C960 *lance;

You still haven't fix your email client. You should notice the tabs have
been turned into spaces.

Please make sure you fix this (and other issues) before sending out the
patch again.

>     struct ariadne_private *priv;
>     int csr0, boguscnt;
>     int handled = 0;
> @@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
>         printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
>         return IRQ_NONE;
>     }
> +     lance = (struct Am79C960 *)dev->base_addr;
> 
>     lance->RAP = CSR0;                     /* PCnet-ISA Controller Status */
> 
> 
> 
> 
> -- 

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  2:50 ` Li Zefan
@ 2011-03-03  8:11   ` Wolfram Sang
  2011-03-03 16:41     ` Randy Dunlap
  2011-03-03 16:42   ` Randy Dunlap
  1 sibling, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2011-03-03  8:11 UTC (permalink / raw)
  To: Li Zefan; +Cc: Jinqiu, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]

Hi,

> Don't ever mention "bug #13853", because this number makes no sense
> for everyone except you. A proper subject may be:
> 
> [PATCH] ariadne: fix possible null dereference.

I agree for the subject line. Still, in the body or maybe after the "---" I'd
think it makes sense, so it is known there is a bugzilla entry to close? Also,
using search engines, one will be able to dig up the patch.

> And please send your patches to proper maintainers and mailing list,
> David Miller and netdev@vger.kernel.org in particular.

Yup. Using scripts/get_maintainers.pl is the way to go here.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  8:11   ` Wolfram Sang
@ 2011-03-03 16:41     ` Randy Dunlap
  2011-03-04  2:03       ` Li Zefan
  0 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2011-03-03 16:41 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Li Zefan, Jinqiu, linux-kernel

On Thu, 3 Mar 2011 09:11:56 +0100 Wolfram Sang wrote:

> Hi,
> 
> > Don't ever mention "bug #13853", because this number makes no sense
> > for everyone except you. A proper subject may be:
> > 
> > [PATCH] ariadne: fix possible null dereference.
> 
> I agree for the subject line. Still, in the body or maybe after the "---" I'd
> think it makes sense, so it is known there is a bugzilla entry to close? Also,
> using search engines, one will be able to dig up the patch.

It should just refer to the full bugzilla #13853 URL:
https://bugzilla.kernel.org/show_bug.cgi?id=13853

> > And please send your patches to proper maintainers and mailing list,
> > David Miller and netdev@vger.kernel.org in particular.
> 
> Yup. Using scripts/get_maintainers.pl is the way to go here.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  2:50 ` Li Zefan
  2011-03-03  8:11   ` Wolfram Sang
@ 2011-03-03 16:42   ` Randy Dunlap
  1 sibling, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2011-03-03 16:42 UTC (permalink / raw)
  To: Li Zefan; +Cc: Jinqiu, linux-kernel

On Thu, 03 Mar 2011 10:50:02 +0800 Li Zefan wrote:

> So you've been told to read Documentation/SubmittingPatches
> and Documentation/email-clients.txt, but there are still quite
> a few problems in your patch..
> 
> Don't ever mention "bug #13853", because this number makes no sense
> for everyone except you. A proper subject may be:
> 
> [PATCH] ariadne: fix possible null dereference.
> 
> And please send your patches to proper maintainers and mailing list,
> David Miller and netdev@vger.kernel.org in particular.
> 
> Jinqiu wrote:
> > This patch fixes bug #13853 .This still exists in latest version.
> 
> Of course it exists, otherwise your patch won't be applied, so just
> remove this line from your changelog.
> 
> > 'dev' is dereferenced before testing for NULL.
> > 
> > Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
> > --- a/drivers/net/ariadne.c
> > +++ b/drivers/net/ariadne.c
> > @@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
> > static irqreturn_t ariadne_interrupt(int irq, void *data)
> > {
> >     struct net_device *dev = (struct net_device *) data;
> > -     volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
> > +     volatile struct Am79C960 *lance;
> 
> You still haven't fix your email client. You should notice the tabs have
> been turned into spaces.

Uh, ariadne.c does not use tabs to begin its lines... it uses spaces.  :(

> Please make sure you fix this (and other issues) before sending out the
> patch again.
> 
> >     struct ariadne_private *priv;
> >     int csr0, boguscnt;
> >     int handled = 0;
> > @@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
> >         printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
> >         return IRQ_NONE;
> >     }
> > +     lance = (struct Am79C960 *)dev->base_addr;
> > 
> >     lance->RAP = CSR0;                     /* PCnet-ISA Controller Status */
> > 
> > 
> > 
> > 
> > -- 


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  2:20 [patch] Fix bug #13853: dereference pointer 'dev' before null check Jinqiu
  2011-03-03  2:50 ` Li Zefan
@ 2011-03-03 19:23 ` Randy Dunlap
  2011-03-03 19:29 ` Randy Dunlap
  2 siblings, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2011-03-03 19:23 UTC (permalink / raw)
  To: Jinqiu; +Cc: linux-kernel

On Wed, 02 Mar 2011 21:20:21 -0500 Jinqiu wrote:

> This patch fixes bug #13853 .This still exists in latest version.
> 'dev' is dereferenced before testing for NULL.
> 
> Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
> --- a/drivers/net/ariadne.c
> +++ b/drivers/net/ariadne.c
> @@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
> static irqreturn_t ariadne_interrupt(int irq, void *data)
> {

It looks like thunderbird has split a long line into multiple lines for you.
This is bad.  See if Documentation/email-clients.txt can help you out any.


> 	struct net_device *dev = (struct net_device *) data;
> - 	volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
> + 	volatile struct Am79C960 *lance;
> 	struct ariadne_private *priv;
> 	int csr0, boguscnt;
> 	int handled = 0;
> @@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
> 		printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
> 		return IRQ_NONE;
> 	}
> + 	lance = (struct Am79C960 *)dev->base_addr;
> 
> 	lance->RAP = CSR0; 					/* PCnet-ISA Controller Status */
> 
> 
> 
> 
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03  2:20 [patch] Fix bug #13853: dereference pointer 'dev' before null check Jinqiu
  2011-03-03  2:50 ` Li Zefan
  2011-03-03 19:23 ` Randy Dunlap
@ 2011-03-03 19:29 ` Randy Dunlap
  2011-03-03 21:19   ` Jinqiu
  2 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2011-03-03 19:29 UTC (permalink / raw)
  To: Jinqiu; +Cc: linux-kernel

On Wed, 02 Mar 2011 21:20:21 -0500 Jinqiu wrote:

> This patch fixes bug #13853 .This still exists in latest version.
> 'dev' is dereferenced before testing for NULL.
> 
> Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
> --- a/drivers/net/ariadne.c
> +++ b/drivers/net/ariadne.c
> @@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
> static irqreturn_t ariadne_interrupt(int irq, void *data)
> {
> 	struct net_device *dev = (struct net_device *) data;
> - 	volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
> + 	volatile struct Am79C960 *lance;
> 	struct ariadne_private *priv;
> 	int csr0, boguscnt;
> 	int handled = 0;

Did you patch generator tool insert tabs in this patch?

The line below should be formatted as:

@@ -429,6 +429,7 @@ static irqreturn_t ariadne_interrupt(int

with no space between 429, and 7.

> @@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
> 		printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
> 		return IRQ_NONE;
> 	}
> + 	lance = (struct Am79C960 *)dev->base_addr;
> 
> 	lance->RAP = CSR0; 					/* PCnet-ISA Controller Status */
> 
> 
> 
> 
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03 19:29 ` Randy Dunlap
@ 2011-03-03 21:19   ` Jinqiu
  0 siblings, 0 replies; 9+ messages in thread
From: Jinqiu @ 2011-03-03 21:19 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-kernel


On 11-3-3 下午2:29, Randy Dunlap wrote:
> On Wed, 02 Mar 2011 21:20:21 -0500 Jinqiu wrote:
>
>> This patch fixes bug #13853 .This still exists in latest version.
>> 'dev' is dereferenced before testing for NULL.
>>
>> Signed-off-by: Jinqiu Yang<crindy646@gmail.com>
>> --- a/drivers/net/ariadne.c
>> +++ b/drivers/net/ariadne.c
>> @@ -420,7 +420,7 @@ static inline void ariadne_reset(struct
>> static irqreturn_t ariadne_interrupt(int irq, void *data)
>> {
>> 	struct net_device *dev = (struct net_device *) data;
>> - 	volatile struct Am79C960 *lance = (struct Am79C960*)dev->base_addr;
>> + 	volatile struct Am79C960 *lance;
>> 	struct ariadne_private *priv;
>> 	int csr0, boguscnt;
>> 	int handled = 0;
> Did you patch generator tool insert tabs in this patch?
No, I did not. This is weird. The original patch is fine.
  I would fix email client issues.
Thanks for your help! :-)

  Jinqiu
> The line below should be formatted as:
>
> @@ -429,6 +429,7 @@ static irqreturn_t ariadne_interrupt(int
>
> with no space between 429, and 7.
>
>> @@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
>> 		printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
>> 		return IRQ_NONE;
>> 	}
>> + 	lance = (struct Am79C960 *)dev->base_addr;
>>
>> 	lance->RAP = CSR0; 					/* PCnet-ISA Controller Status */
>>
>>
>>
>>
>> --
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***


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

* Re: [patch] Fix bug #13853: dereference pointer 'dev' before null check
  2011-03-03 16:41     ` Randy Dunlap
@ 2011-03-04  2:03       ` Li Zefan
  0 siblings, 0 replies; 9+ messages in thread
From: Li Zefan @ 2011-03-04  2:03 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Wolfram Sang, Jinqiu, linux-kernel

Randy Dunlap wrote:
> On Thu, 3 Mar 2011 09:11:56 +0100 Wolfram Sang wrote:
> 
>> Hi,
>>
>>> Don't ever mention "bug #13853", because this number makes no sense
>>> for everyone except you. A proper subject may be:
>>>
>>> [PATCH] ariadne: fix possible null dereference.
>>
>> I agree for the subject line. Still, in the body or maybe after the "---" I'd
>> think it makes sense, so it is known there is a bugzilla entry to close? Also,
>> using search engines, one will be able to dig up the patch.
> 
> It should just refer to the full bugzilla #13853 URL:
> https://bugzilla.kernel.org/show_bug.cgi?id=13853
> 

Ok, then this should be made clear, for example add the URL in the changelog.


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

end of thread, other threads:[~2011-03-04  2:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-03  2:20 [patch] Fix bug #13853: dereference pointer 'dev' before null check Jinqiu
2011-03-03  2:50 ` Li Zefan
2011-03-03  8:11   ` Wolfram Sang
2011-03-03 16:41     ` Randy Dunlap
2011-03-04  2:03       ` Li Zefan
2011-03-03 16:42   ` Randy Dunlap
2011-03-03 19:23 ` Randy Dunlap
2011-03-03 19:29 ` Randy Dunlap
2011-03-03 21:19   ` Jinqiu

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