Fix bug #13853: dereference pointer 'dev' before null check

PATCH In review Jinqiu 4 participants 9 messages Mar 3, 2011 – Mar 4, 2011 Network drivers

lore

Share & tools

lore

Jinqiu

author

2011-03-03 02:20 UTC #0 lore raw

PATCH 1/1 Fix bug #13853: dereference pointer 'dev' before null check

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

Signed-off-by Jinqiu Yang<[email protected]>

drivers/net/ariadne.c +0 -0

@@ -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 */

Li Zefan

2011-03-03 02:48 UTC #1 lore raw

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 [email protected] in particular.

Quoted 1 line · Jinqiu

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.

drivers/net/ariadne.c static inline void ariadne_reset(struct · line 423

Quoted 11 lines

'dev' is dereferenced before testing for NULL.

Signed-off-by Jinqiu Yang<[email protected]>

drivers/net/ariadne.c +1 -1

@@ -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.

Quoted 15 lines

drivers/net/ariadne.c +0 -0

    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 */

Wolfram Sang

2011-03-03 08:12 UTC #2 lore raw

Hi,

Quoted 4 lines

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.

Quoted 2 lines

And please send your patches to proper maintainers and mailing list, David Miller and [email protected] in particular.

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

Thanks,

Wolfram

Signature · 2 lines

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

Randy Dunlap

2011-03-03 16:41 UTC #3 lore raw

Quoted 10 lines · On Thu, 3 Mar 2011 09:11:56 +0100 Wolfram Sang wrote:

Hi,
Quoted 4 lines
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

Quoted 4 lines

Quoted 2 lines
And please send your patches to proper maintainers and mailing list, David Miller and [email protected] in particular.
Yup. Using scripts/get_maintainers.pl is the way to go here.

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

Li Zefan

2011-03-04 02:01 UTC #4 lore raw

Quoted 7 lines · Randy Dunlap

Quoted 5 lines · On Thu, 3 Mar 2011 09:11:56 +0100 Wolfram Sang wrote:
Hi,
Quoted 3 lines
Don't ever mention "bug #13853", because this number makes no sense for everyone except you. A proper subject may be:

Quoted 9 lines

Quoted 5 lines
Quoted 1 line
[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.

Randy Dunlap

2011-03-03 16:42 UTC #5 lore raw

In reply to Li Zefan

Quoted 20 lines · 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 [email protected] in particular.
Quoted 1 line · Jinqiu
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.
Quoted 2 lines
'dev' is dereferenced before testing for NULL.

Quoted 12 lines

drivers/net/ariadne.c static inline void ariadne_reset(struct · line 423
Quoted 9 lines
Signed-off-by Jinqiu Yang<[email protected]>
drivers/net/ariadne.c +1 -1
@@ -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. :(

Quoted 12 lines

Please make sure you fix this (and other issues) before sending out the patch again.
Quoted 9 lines
drivers/net/ariadne.c +0 -0
    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;

Quoted 6 lines

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

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

Randy Dunlap

2011-03-03 19:23 UTC #6 lore raw

In reply to Jinqiu

drivers/net/ariadne.c static inline void ariadne_reset(struct · line 421

Quoted 9 lines · 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<[email protected]>
drivers/net/ariadne.c +0 -0
@@ -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.

Quoted 12 lines

drivers/net/ariadne.c +1 -1

 	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;
	}

drivers/net/ariadne.c +1 -0

+ 	lance = (struct Am79C960 *)dev->base_addr;

Quoted 6 lines

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

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

Randy Dunlap

2011-03-03 19:29 UTC #7 lore raw

In reply to Jinqiu

Quoted 7 lines · 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<[email protected]>

drivers/net/ariadne.c static inline void ariadne_reset(struct · line 426

Quoted 8 lines

drivers/net/ariadne.c +1 -1

@@ -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.

Quoted 6 lines

@@ -429,6 +429, 7 @@ static irqreturn_t ariadne_interrupt(int
		printk(KERN_WARNING  "ariadne_interrupt(): irq for unknown device.\n");
		return IRQ_NONE;
	}
drivers/net/ariadne.c +1 -0
+ 	lance = (struct Am79C960 *)dev->base_addr;

Quoted 6 lines

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

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

Jinqiu

author

2011-03-03 21:19 UTC #8 lore raw

Quoted 5 lines · Randy Dunlap

Quoted 3 lines · 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.

Quoted 13 lines

drivers/net/ariadne.c static inline void ariadne_reset(struct · line 426
Quoted 12 lines
Signed-off-by Jinqiu Yang<[email protected]>
drivers/net/ariadne.c +1 -1
@@ -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

Quoted 22 lines

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