2003-08-30 19:55:40

by Luca

[permalink] [raw]
Subject: [PATCH] Use after free in drivers/media/video/videodev.c

Hi,
I think that there's a bug in videdev.c. Look at
video_unregister_device:

void video_unregister_device(struct video_device *vfd) {
[...]
class_device_unregister(&vfd->class_dev);
devfs_remove(vfd->devfs_name);
video_device[vfd->minor]=NULL;
}

The class_device_unregister will call video_release. This function will
call a ->release callback. As far as I can see drivers do their
own cleanup outside video_unregister_device so there is no problem.

However, if a driver switch to dynamically allocated video_device
this ->release callback will free the struct video_device (look
at video_device_release) and possibly its container. So after
class_device_unregister vfd may be a pointer to deallocated memory.

I think that class_device_unregister should be moved down:

--- 2.6.0.orig/drivers/media/video/videodev.c Tue Aug 12 17:02:29 2003
+++ 2.6.0/drivers/media/video/videodev.c Sat Aug 30 21:13:29 2003
@@ -349,9 +349,9 @@
if(video_device[vfd->minor]!=vfd)
panic("videodev: bad unregister");

- class_device_unregister(&vfd->class_dev);
devfs_remove(vfd->devfs_name);
video_device[vfd->minor]=NULL;
+ class_device_unregister(&vfd->class_dev);
up(&videodev_lock);
}


Luca
--
Reply-To: [email protected]
Home: http://kronoz.cjb.net
The trouble with computers is that they do what you tell them,
not what you want.
D. Cohen


2003-08-30 22:17:24

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Use after free in drivers/media/video/videodev.c

Kronos <[email protected]> wrote:
>
> --- 2.6.0.orig/drivers/media/video/videodev.c Tue Aug 12 17:02:29 2003
> +++ 2.6.0/drivers/media/video/videodev.c Sat Aug 30 21:13:29 2003
> @@ -349,9 +349,9 @@
> if(video_device[vfd->minor]!=vfd)
> panic("videodev: bad unregister");
>
> - class_device_unregister(&vfd->class_dev);
> devfs_remove(vfd->devfs_name);
> video_device[vfd->minor]=NULL;
> + class_device_unregister(&vfd->class_dev);
> up(&videodev_lock);
> }

Yes. I already have this fix (from gregkh) queued up.

There are around 150 bugfixes in -mm at present. It is worth
checking there first...