Skip to contentSkip to conversation

rapidio: potential null deref on failure path

PATCH In review Dan Carpenter 2 participants 7 messages Mar 19, 2011 – Mar 21, 2011

Subsystems (2)
Kernel janitors Rapidio
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 rapidio: potential null deref on failure path

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

Signed-off-by Dan Carpenter <[email protected]>
drivers/rapidio/rio-scan.c +1 -1
index a50391b..3a59d5f 100644
@@ -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);
#1 lore raw

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]>
drivers/rapidio/rio-scan.c +0 -0
index a50391b..3a59d5f 100644
@@ -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);

author
#2 lore raw
Quoted 8 lines · Alexandre

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

#3 lore raw

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

Quoted 8 lines · Alexandre

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

author
#4 lore raw
Quoted 3 lines · Alexandre

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

#5 lore raw

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

Quoted 3 lines · Alexandre

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

Acked-by on v1 1/1
#6 lore raw
In reply to Dan Carpenter
Quoted 24 lines

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]>
drivers/rapidio/rio-scan.c +1 -1
index a50391b..3a59d5f 100644
@@ -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);
Quoted 1 line

kfree(rdev);

Acked-by Alexandre Bounine <[email protected]>