2007-08-14 21:28:44

by Adrian Bunk

Subject: [2.6 patch] w1_remove_master_device(): fix check-after-use

The Coverity checker spotted that we'd have already oops'ed if "dev"
was NULL.

Signed-off-by: Adrian Bunk <[email protected]>

---
--- linux-2.6.23-rc1-mm2/drivers/w1/w1_int.c.old 2007-08-08 14:38:50.000000000 +0200
+++ linux-2.6.23-rc1-mm2/drivers/w1/w1_int.c 2007-08-08 14:38:58.000000000 +0200
@@ -171,22 +171,17 @@
void w1_remove_master_device(struct w1_bus_master *bm)
{
struct w1_master *dev = NULL;

list_for_each_entry(dev, &w1_masters, w1_master_entry) {
if (!dev->initialized)
continue;

if (dev->bus_master->data == bm->data)
break;
}

- if (!dev) {
- printk(KERN_ERR "Device doesn't exist.\n");
- return;
- }
-
__w1_remove_master_device(dev);
}

EXPORT_SYMBOL(w1_add_master_device);
EXPORT_SYMBOL(w1_remove_master_device);

2007-08-14 22:24:51

by Evgeniy Polyakov

Subject: Re: [2.6 patch] w1_remove_master_device(): fix check-after-use

Hi Adrian.

On Tue, Aug 14, 2007 at 11:22:48PM +0200, Adrian Bunk ([email protected]) wrote:
> The Coverity checker spotted that we'd have already oops'ed if "dev"
> was NULL.

This is wrong.
Although dev can not be null there there is no way it will crash.
The right paranoidal solution is to setup new pointer and make it equal
to the found device and check if it is NULL or not out of the loop.
I will cook up a patch tomorrow, thanks for pointing to this issue.

--
Evgeniy Polyakov