Messages in this thread Patch in this message | | | Date | Mon, 21 Nov 2005 00:52:42 +0100 | From | Adrian Bunk <> | Subject | Re: [2.6 patch] drivers/ieee1394/raw1394.c: fix a NULL pointer dereference |
| |
On Sun, Nov 20, 2005 at 06:40:55PM -0500, Dave Jones wrote: > On Mon, Nov 21, 2005 at 12:20:09AM +0100, Adrian Bunk wrote: > > The coverity checker spotted that this was a NULL pointer dereference in > > the "if (copy_from_user(...))" case. > > > > > > Signed-off-by: Adrian Bunk <bunk@stusta.de> > > > > --- linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c.old 2005-11-20 22:08:57.000000000 +0100 > > +++ linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c 2005-11-20 22:09:34.000000000 +0100 > > @@ -2166,7 +2166,8 @@ > > } > > } > > } > > - kfree(cache->filled_head); > > + if(cache->filled_head) > > + kfree(cache->filled_head); > > kfree(cache); > > > > if (ret >= 0) { > > > > How do we get that far with a NULL filled_head ? > If the kmalloc that fills cache->filled_head fails, we bail out early above.
The problem is not a NULL filled_head.
The problem is that in the "if (copy_from_user(...))" case, cache has already been freed.
But thinking about this, my patch is also wrong and creates a memory leak and the real bug is the freeing of cache above.
What about the patch below?
> Dave
cu Adrian
<-- snip -->
The coverity checker spotted that this was a NULL pointer dereference in the "if (copy_from_user(...))" case since the next step is to kfree(cache->filled_head).
There's no need to free cache at this point, and it's getting free'd later.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c.old 2005-11-20 22:08:57.000000000 +0100 +++ linux-2.6.15-rc1-mm2-full/drivers/ieee1394/raw1394.c 2005-11-21 00:49:38.000000000 +0100 @@ -2131,7 +2131,6 @@ req->req.length)) { csr1212_release_keyval(fi->csr1212_dirs[dr]); fi->csr1212_dirs[dr] = NULL; - CSR1212_FREE(cache); ret = -EFAULT; } else { cache->len = req->req.length; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |