Re: [SECURITY] CAN-2004-0075

Michal Schmidt
2004-04-15T10:04:51Z

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Greg KH wrote:
> On Wed, Apr 14, 2004 at 10:30:33PM +0200, Marc-Christian Petersen wrote:
>>Okay, now while we are at fixing security holes, is there any chance we
>>can _finally_ get the attached patch in?
>>
>>The Vicam USB driver in all Linux Kernels 2.6 mainline does not use the
>>copy_from_user function when copying data from userspace to kernel space,
>>which crosses security boundaries and allows local users to cause a denial
>>of service.
>>
>>Already ACKed by Greg. Only complaint was inproper coding style which is done
>>with attached patch ;)
>
>
> Eeek, I thought this one was already in the tree, very sorry about that.
>
> I'm applying it now and will send it to Linus in a bit.
>

The patch broke compilation with VICAM_DEBUG on.
There is also another copy_from_user missing in case VIDIOCSPICT.
I'm attaching a patch.

Michal Schmidt

[-- Attachment #2: vicam-ioctl.diff --]
[-- Type: text/plain, Size: 1125 bytes --]

--- linux-2.6.6-rc1/drivers/usb/media/vicam.c 2004-04-15 11:18:18.000000000 +0200
+++ linux-2.6.6-rc1-mich/drivers/usb/media/vicam.c 2004-04-15 11:50:02.791604312 +0200
@@ -612,15 +612,20 @@ vicam_ioctl(struct inode *inode, struct

case VIDIOCSPICT:
{
- struct video_picture *vp = (struct video_picture *) arg;
-
- DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
- vp->palette);
+ struct video_picture vp;
+
+ if (copy_from_user(&vp, arg, sizeof(vp))) {
+ retval = -EFAULT;
+ break;
+ }
+
+ DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp.depth,
+ vp.palette);

- cam->gain = vp->brightness >> 8;
+ cam->gain = vp.brightness >> 8;

- if (vp->depth != 24
- || vp->palette != VIDEO_PALETTE_RGB24)
+ if (vp.depth != 24
+ || vp.palette != VIDEO_PALETTE_RGB24)
retval = -EINVAL;

break;
@@ -660,7 +665,7 @@ vicam_ioctl(struct inode *inode, struct
break;
}

- DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
+ DBG("VIDIOCSWIN %d x %d\n", vw.width, vw.height);

if ( vw.width != 320 || vw.height != 240 )
retval = -EFAULT;