[PATCH] Use after free in drivers/media/video/videodev.c

[PATCH] Use after free in drivers/media/video/videodev.c

[Kronos]

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: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
The trouble with computers is that they do what you tell them,
not what you want.
D. Cohen

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

[Andrew Morton]

Kronos <kronos@kronoz.cjb.net> 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...