From: Jesper Juhl Date: Fri, 23 Jun 2006 16:27:36 +0000 (-0300) Subject: V4L/DVB (4154): Fix use-after-free bug in cpia2 driver X-Git-Tag: v2.6.18-rc1~862^2~50 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bafefc0cf8e4b34fbb159ea2e2aef2358ebff935;p=linux-2.6 V4L/DVB (4154): Fix use-after-free bug in cpia2 driver The coverity checker detected a use-after-free error in drivers/media/video/cpia2/cpia2_v4l.c::cpia2_close() (coverity error #1281). What happens is that we lock cam->busy_lock, then proceed to free resources, and in the case of (--cam->open_count == 0) we finish off by doing a kfree(cam) and then at the end of the function we do a mutex_unlock(&cam->busy_lock) which will explode since it'll dereference the free'd `cam' : ... mutex_lock(&cam->busy_lock); ... if (--cam->open_count == 0) { ... if (!cam->present) { video_unregister_device(dev); kfree(cam); } } mutex_unlock(&cam->busy_lock); <--- PROBLEM, cam no longer around. ... Since this only happens in the case of open_count going down to zero I don't see a problem with just releasing the mutex after unregistering the device and just before the kfree(). In this case there is nothing around that we can race against; we are in the release method, open_count is zero, (!cam->present) and the device has just been unregistered, so letting go of the mutex at this point looks safe to me. Patch below to implement that solution. Acked-by: Randy Dunlap Signed-off-by: Jesper Juhl Signed-off-by: Andrew Morton Signed-off-by: Mauro Carvalho Chehab --- diff --git a/drivers/media/video/cpia2/cpia2_v4l.c b/drivers/media/video/cpia2/cpia2_v4l.c index 28d93c595d..d129db57fc 100644 --- a/drivers/media/video/cpia2/cpia2_v4l.c +++ b/drivers/media/video/cpia2/cpia2_v4l.c @@ -343,7 +343,9 @@ static int cpia2_close(struct inode *inode, struct file *file) cpia2_free_buffers(cam); if (!cam->present) { video_unregister_device(dev); + mutex_unlock(&cam->busy_lock); kfree(cam); + return 0; } }