[linux-dvb] Re: vdr / OSDSetPalette oops with 2.6.x

[Johannes Stezenbach <js@convergence.de>]

Pekka Pietikainen wrote:
> I've poked around the bug mentioned in
> http://www.linuxtv.org:81/mailinglists/linux-dvb/2004/05-2004/msg00445.html
> I can easily see why it does that...
> 
> static int dvb_osd_ioctl(struct inode *inode, struct file *file,
>                          unsigned int cmd, void *parg)
> {
>         struct dvb_device *dvbdev = (struct dvb_device *) file->private_data;
>         struct av7110 *av7110 = (struct av7110 *) dvbdev->priv;
>  
>         DEB_EE(("av7110: %p\n", av7110));
>  
>         if (cmd == OSD_SEND_CMD)
>                 return av7110_osd_cmd(av7110, (osd_cmd_t *) parg);
>  
>         return -EINVAL;
> }
> 
> which is obviously missing a copy_from_user(&karg,parg,sizeof(osd_cmd_t))

Nope, that already happens in dvb_usercopy().

> and if data != NULL like it is in OSD_SetPalette or OSD_SetBlock 
> copy_from_user() that as well. 
> 
> I think this requires API changes to really work, currently osd_cmd_t is
> 
> typedef struct osd_cmd_s {
>         OSD_Command cmd;
>         int x0;
>         int y0;
>         int x1;
>         int y1;
>         int color;
>         void *data;
> } osd_cmd_t;
> 
> so there's no way for the kernel to know how long data is so it can copy 
> it. It definately can't use the pointer provided by userspace directly even
> though that might have worked previously.

OSD_SetBlock() does the required copy_from_user(). the size of *data
is infered from the other paramters.

OSD_SetPalette() is buggy, however, and it seems it always was, even
in the original implementation in the old "DVB" driver.

No API change required, however, if copy_from_user() fails the ioctl
returns -EFAULT.

Will someone send me patches for OSD_SetPalette() for DVB and dvb-kernel
2.4 and 2.6?


Johannes