* [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 13:38 ` Julia Lawall
0 siblings, 0 replies; 14+ messages in thread
From: Julia Lawall @ 2008-05-12 13:38 UTC (permalink / raw)
To: pantelis.antoniou, vbordug, linuxppc-dev, netdev, linux-kernel,
kernel-janitors
From: Julia Lawall <julia@diku.dk>
The following code appears in the function fs_init_instance in the file drivers/net/fs_enet/fs_enet-main.c.
if (fep->ops == NULL) {
printk(KERN_ERR DRV_MODULE_NAME
": %s No matching ops found (%d).\n",
ndev->name, fpi->fs_no);
err = -EINVAL;
goto err;
}
This code implies that at the point of err, fep->ops can be NULL, so an
extra test is needed before dereferencing this value.
This problem was found using the following semantic match
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@@
expression E, E1;
identifier f;
statement S1,S2,S3;
@@
* if (E == NULL)
{
... when != if (E == NULL) S1 else S2
when != E = E1
* E->f
... when any
return ...;
}
else S3
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
--- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
+++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
@@ -1093,7 +1093,7 @@ err:
if (registered)
unregister_netdev(ndev);
- if (fep != NULL) {
+ if (fep && fep->ops) {
(*fep->ops->free_bd)(ndev);
(*fep->ops->cleanup_data)(ndev);
}
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer
2008-05-12 13:38 ` Julia Lawall
(?)
@ 2008-05-12 15:37 ` Vitaly Bordug
-1 siblings, 0 replies; 14+ messages in thread
From: Vitaly Bordug @ 2008-05-12 15:37 UTC (permalink / raw)
To: Julia Lawall; +Cc: linuxppc-dev, netdev, kernel-janitors, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On Mon, 12 May 2008 15:38:26 +0200 (CEST)
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
>
> The following code appears in the function fs_init_instance in the
> file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops == NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so
> an extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E == NULL)
> {
> ... when != if (E == NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
>
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c ---
> a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27
> 11:41:11.000000000 +0200 +++
> b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12
> 09:41:52.000000000 +0200 @@ -1093,7 +1093,7 @@ err: if (registered)
> unregister_netdev(ndev);
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 15:37 ` Vitaly Bordug
0 siblings, 0 replies; 14+ messages in thread
From: Vitaly Bordug @ 2008-05-12 15:37 UTC (permalink / raw)
To: Julia Lawall
Cc: pantelis.antoniou, linuxppc-dev, netdev, linux-kernel,
kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On Mon, 12 May 2008 15:38:26 +0200 (CEST)
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
>
> The following code appears in the function fs_init_instance in the
> file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops == NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so
> an extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E == NULL)
> {
> ... when != if (E == NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
>
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c ---
> a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27
> 11:41:11.000000000 +0200 +++
> b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12
> 09:41:52.000000000 +0200 @@ -1093,7 +1093,7 @@ err: if (registered)
> unregister_netdev(ndev);
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 15:37 ` Vitaly Bordug
0 siblings, 0 replies; 14+ messages in thread
From: Vitaly Bordug @ 2008-05-12 15:37 UTC (permalink / raw)
To: Julia Lawall; +Cc: linuxppc-dev, netdev, kernel-janitors, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On Mon, 12 May 2008 15:38:26 +0200 (CEST)
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
>
> The following code appears in the function fs_init_instance in the
> file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops == NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so
> an extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E == NULL)
> {
> ... when != if (E == NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
>
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c ---
> a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27
> 11:41:11.000000000 +0200 +++
> b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12
> 09:41:52.000000000 +0200 @@ -1093,7 +1093,7 @@ err: if (registered)
> unregister_netdev(ndev);
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer
2008-05-12 13:38 ` Julia Lawall
(?)
@ 2008-05-12 16:51 ` Francois Romieu
-1 siblings, 0 replies; 14+ messages in thread
From: Francois Romieu @ 2008-05-12 16:51 UTC (permalink / raw)
To: Julia Lawall
Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, vbordug,
jgarzik
Julia Lawall <julia@diku.dk> :
[...]
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
> @@ -1093,7 +1093,7 @@ err:
> if (registered)
> unregister_netdev(ndev);
>
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
Extra cookies for the nice soul who:
- removes the 'if (registered)' test (it can not happen)
- uses different error lablels and unrolls the error path. I can not claim
that the current error path is wrong but it would not hurt if it was more
trivially balanced.
--
Ueimor
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 16:51 ` Francois Romieu
0 siblings, 0 replies; 14+ messages in thread
From: Francois Romieu @ 2008-05-12 16:51 UTC (permalink / raw)
To: Julia Lawall
Cc: pantelis.antoniou, vbordug, linuxppc-dev, netdev, linux-kernel,
kernel-janitors, jgarzik
Julia Lawall <julia@diku.dk> :
[...]
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
> @@ -1093,7 +1093,7 @@ err:
> if (registered)
> unregister_netdev(ndev);
>
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
Extra cookies for the nice soul who:
- removes the 'if (registered)' test (it can not happen)
- uses different error lablels and unrolls the error path. I can not claim
that the current error path is wrong but it would not hurt if it was more
trivially balanced.
--
Ueimor
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 16:51 ` Francois Romieu
0 siblings, 0 replies; 14+ messages in thread
From: Francois Romieu @ 2008-05-12 16:51 UTC (permalink / raw)
To: Julia Lawall
Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, vbordug,
jgarzik
Julia Lawall <julia@diku.dk> :
[...]
> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
> @@ -1093,7 +1093,7 @@ err:
> if (registered)
> unregister_netdev(ndev);
>
> - if (fep != NULL) {
> + if (fep && fep->ops) {
> (*fep->ops->free_bd)(ndev);
> (*fep->ops->cleanup_data)(ndev);
> }
Extra cookies for the nice soul who:
- removes the 'if (registered)' test (it can not happen)
- uses different error lablels and unrolls the error path. I can not claim
that the current error path is wrong but it would not hurt if it was more
trivially balanced.
--
Ueimor
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
2008-05-12 16:51 ` Francois Romieu
(?)
@ 2008-05-12 17:22 ` Scott Wood
-1 siblings, 0 replies; 14+ messages in thread
From: Scott Wood @ 2008-05-12 17:22 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, Julia Lawall,
vbordug, jgarzik
Francois Romieu wrote:
> Julia Lawall <julia@diku.dk> :
> [...]
>> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
>> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
>> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
>> @@ -1093,7 +1093,7 @@ err:
>> if (registered)
>> unregister_netdev(ndev);
>>
>> - if (fep != NULL) {
>> + if (fep && fep->ops) {
>> (*fep->ops->free_bd)(ndev);
>> (*fep->ops->cleanup_data)(ndev);
>> }
>
> Extra cookies for the nice soul who:
> - removes the 'if (registered)' test (it can not happen)
> - uses different error lablels and unrolls the error path. I can not claim
> that the current error path is wrong but it would not hurt if it was more
> trivially balanced.
>
Please note that this code is going away very soon, when arch/ppc dies.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 17:22 ` Scott Wood
0 siblings, 0 replies; 14+ messages in thread
From: Scott Wood @ 2008-05-12 17:22 UTC (permalink / raw)
To: Francois Romieu
Cc: Julia Lawall, netdev, kernel-janitors, linux-kernel, linuxppc-dev,
vbordug, jgarzik
Francois Romieu wrote:
> Julia Lawall <julia@diku.dk> :
> [...]
>> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
>> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
>> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
>> @@ -1093,7 +1093,7 @@ err:
>> if (registered)
>> unregister_netdev(ndev);
>>
>> - if (fep != NULL) {
>> + if (fep && fep->ops) {
>> (*fep->ops->free_bd)(ndev);
>> (*fep->ops->cleanup_data)(ndev);
>> }
>
> Extra cookies for the nice soul who:
> - removes the 'if (registered)' test (it can not happen)
> - uses different error lablels and unrolls the error path. I can not claim
> that the current error path is wrong but it would not hurt if it was more
> trivially balanced.
>
Please note that this code is going away very soon, when arch/ppc dies.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-12 17:22 ` Scott Wood
0 siblings, 0 replies; 14+ messages in thread
From: Scott Wood @ 2008-05-12 17:22 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, Julia Lawall,
vbordug, jgarzik
Francois Romieu wrote:
> Julia Lawall <julia@diku.dk> :
> [...]
>> diff -u -p a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
>> --- a/drivers/net/fs_enet/fs_enet-main.c 2008-04-27 11:41:11.000000000 +0200
>> +++ b/drivers/net/fs_enet/fs_enet-main.c 2008-05-12 09:41:52.000000000 +0200
>> @@ -1093,7 +1093,7 @@ err:
>> if (registered)
>> unregister_netdev(ndev);
>>
>> - if (fep != NULL) {
>> + if (fep && fep->ops) {
>> (*fep->ops->free_bd)(ndev);
>> (*fep->ops->cleanup_data)(ndev);
>> }
>
> Extra cookies for the nice soul who:
> - removes the 'if (registered)' test (it can not happen)
> - uses different error lablels and unrolls the error path. I can not claim
> that the current error path is wrong but it would not hurt if it was more
> trivially balanced.
>
Please note that this code is going away very soon, when arch/ppc dies.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
2008-05-12 13:38 ` Julia Lawall
(?)
@ 2008-05-22 10:27 ` Jeff Garzik
-1 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2008-05-22 10:27 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, vbordug
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> The following code appears in the function fs_init_instance in the file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops = NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so an
> extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E = NULL)
> {
> ... when != if (E = NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
applied
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-22 10:27 ` Jeff Garzik
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2008-05-22 10:27 UTC (permalink / raw)
To: Julia Lawall
Cc: pantelis.antoniou, vbordug, linuxppc-dev, netdev, linux-kernel,
kernel-janitors
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> The following code appears in the function fs_init_instance in the file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops == NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so an
> extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E == NULL)
> {
> ... when != if (E == NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
applied
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/6] drivers/net/fs_enet: remove null pointer dereference
@ 2008-05-22 10:27 ` Jeff Garzik
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2008-05-22 10:27 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, kernel-janitors, linux-kernel, linuxppc-dev, vbordug
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> The following code appears in the function fs_init_instance in the file drivers/net/fs_enet/fs_enet-main.c.
>
> if (fep->ops == NULL) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s No matching ops found (%d).\n",
> ndev->name, fpi->fs_no);
> err = -EINVAL;
> goto err;
> }
>
> This code implies that at the point of err, fep->ops can be NULL, so an
> extra test is needed before dereferencing this value.
>
>
> This problem was found using the following semantic match
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @@
> expression E, E1;
> identifier f;
> statement S1,S2,S3;
> @@
>
> * if (E == NULL)
> {
> ... when != if (E == NULL) S1 else S2
> when != E = E1
> * E->f
> ... when any
> return ...;
> }
> else S3
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
applied
^ permalink raw reply [flat|nested] 14+ messages in thread