2011-03-19 04:40:57

by Dan Carpenter

[permalink] [raw]
Subject: [patch] rapidio: potential null deref on failure path

If rio is not a switch then "rswitch" is null.

Signed-off-by: Dan Carpenter <[email protected]>

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index a50391b..3a59d5f 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -517,7 +517,7 @@ static struct rio_dev __devinit *rio_setup_device(struct rio_net *net,
return rdev;

cleanup:
- if (rswitch->route_table)
+ if (rio_is_switch(rdev))
kfree(rswitch->route_table);

kfree(rdev);


2011-03-21 12:49:17

by Bounine, Alexandre

[permalink] [raw]
Subject: RE: [patch] rapidio: potential null deref on failure path

Better to combine both conditions:

if (rio_is_switch(rdev) && rswitch->route_table))
kfree(rswitch->route_table);

Because there is situation when routine fails to allocate route_table
for a switch.

Alex.


-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Saturday, March 19, 2011 12:41 AM
To: Matt Porter
Cc: Bounine, Alexandre; [email protected];
[email protected]
Subject: [patch] rapidio: potential null deref on failure path

If rio is not a switch then "rswitch" is null.

Signed-off-by: Dan Carpenter <[email protected]>

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index a50391b..3a59d5f 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -517,7 +517,7 @@ static struct rio_dev __devinit
*rio_setup_device(struct rio_net *net,
return rdev;

cleanup:
- if (rswitch->route_table)
+ if (rio_is_switch(rdev))
kfree(rswitch->route_table);

kfree(rdev);

2011-03-21 15:01:39

by Dan Carpenter

[permalink] [raw]
Subject: Re: [patch] rapidio: potential null deref on failure path

On Mon, Mar 21, 2011 at 05:49:06AM -0700, Bounine, Alexandre wrote:
> Better to combine both conditions:
>
> if (rio_is_switch(rdev) && rswitch->route_table))
> kfree(rswitch->route_table);
>
> Because there is situation when routine fails to allocate route_table
> for a switch.
>

kfree() accepts NULL pointers. People have been removing these checks
all over the tree by the hundreds... Sorry I should have mentioned
that.

regards,
dan carpenter

2011-03-21 15:15:42

by Bounine, Alexandre

[permalink] [raw]
Subject: RE: [patch] rapidio: potential null deref on failure path

In this case I agree.
Do you want me to move this patch up or you have your own path for it?

Alex.

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Monday, March 21, 2011 11:00 AM
To: Bounine, Alexandre
Cc: Matt Porter; [email protected];
[email protected]
Subject: Re: [patch] rapidio: potential null deref on failure path

On Mon, Mar 21, 2011 at 05:49:06AM -0700, Bounine, Alexandre wrote:
> Better to combine both conditions:
>
> if (rio_is_switch(rdev) && rswitch->route_table))
> kfree(rswitch->route_table);
>
> Because there is situation when routine fails to allocate route_table
> for a switch.
>

kfree() accepts NULL pointers. People have been removing these checks
all over the tree by the hundreds... Sorry I should have mentioned
that.

regards,
dan carpenter

2011-03-21 15:53:48

by Dan Carpenter

[permalink] [raw]
Subject: Re: [patch] rapidio: potential null deref on failure path

On Mon, Mar 21, 2011 at 08:15:33AM -0700, Bounine, Alexandre wrote:
> In this case I agree.
> Do you want me to move this patch up or you have your own path for it?
>

Could you take it, please?

regards,
dan carpenter

2011-03-21 16:05:54

by Bounine, Alexandre

[permalink] [raw]
Subject: RE: [patch] rapidio: potential null deref on failure path

No problem.

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Monday, March 21, 2011 11:52 AM
To: Bounine, Alexandre
Cc: Matt Porter; [email protected];
[email protected]
Subject: Re: [patch] rapidio: potential null deref on failure path

On Mon, Mar 21, 2011 at 08:15:33AM -0700, Bounine, Alexandre wrote:
> In this case I agree.
> Do you want me to move this patch up or you have your own path for it?
>

Could you take it, please?

regards,
dan carpenter

2011-03-21 18:35:06

by Bounine, Alexandre

[permalink] [raw]
Subject: RE: [patch] rapidio: potential null deref on failure path

> From: Dan Carpenter [mailto:[email protected]]
> Sent: Saturday, March 19, 2011 12:41 AM
> To: Matt Porter
> Cc: Bounine, Alexandre; [email protected]; kernel-
> [email protected]
> Subject: [patch] rapidio: potential null deref on failure path
>
> If rio is not a switch then "rswitch" is null.
>
> Signed-off-by: Dan Carpenter <[email protected]>
>
> diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
> index a50391b..3a59d5f 100644
> --- a/drivers/rapidio/rio-scan.c
> +++ b/drivers/rapidio/rio-scan.c
> @@ -517,7 +517,7 @@ static struct rio_dev __devinit
> *rio_setup_device(struct rio_net *net,
> return rdev;
>
> cleanup:
> - if (rswitch->route_table)
> + if (rio_is_switch(rdev))
> kfree(rswitch->route_table);
>
> kfree(rdev);

Acked-by: Alexandre Bounine <[email protected]>