On Fri, Jun 11, 2004 at 07:20:25PM +0200, Johannes Stezenbach wrote:
> > which is obviously missing a copy_from_user(&karg,parg,sizeof(osd_cmd_t))
>
> Nope, that already happens in dvb_usercopy().
> 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.
Yup, the multiple levels of indirection and copy_from_user() deep inside
the code in LoadBitmap() confused me. I'm more used to the traditional
foo_ioctl()
{
struct foo_params foo;
if(copy_from_user(&foo,uptr,sizeof(foo))
return -EFAULT
switch(foo->cmd) {
case X:
...
case something_requiring_variable_len_params:
someparams=kmalloc(foo->paramlen,GFP_KERNEL);
if(copy_from_user(someparams,foo->varlendata,foo->paramlen)) {
kfree(someparams)
return -EFAULT
}
do_stuff(&foo,someparams)
}
}
which makes it a lot easier to see what's happening without reading all of
the code, but I suppose it's a matter of taste ;-)
>
> Will someone send me patches for OSD_SetPalette() for DVB and dvb-kernel
> 2.4 and 2.6?
Something like this seems to work. The alpha channel looks a bit funny
in vdr, not sure if it's a vdr, vdr theme or a driver thing tho.
Index: av7110_hw.c
===================================================================
RCS file: /cvs/linuxtv/dvb-kernel/linux/drivers/media/dvb/ttpci/av7110_hw.c,v
retrieving revision 1.11
diff -u -r1.11 av7110_hw.c
--- av7110_hw.c 3 May 2004 16:36:29 -0000 1.11
+++ av7110_hw.c 11 Jun 2004 19:45:07 -0000
@@ -952,17 +952,33 @@
return 0;
case OSD_SetPalette:
{
+ u8 *colors;
+ size_t len;
+
+ len = dc->x0*4;
+
+ if (len > DATA_BUFF3_SIZE)
+ return -EINVAL;
+
+ colors = kmalloc(len,GFP_KERNEL);
+ if(!colors)
+ return -ENOMEM;
+
+ if (copy_from_user(colors,dc->data,len))
+ return -EFAULT;
+
if (FW_VERSION(av7110->arm_app) >= 0x2618)
- OSDSetPalette(av7110, (u32 *)dc->data, dc->color, dc->x0);
- else {
- int i, len = dc->x0-dc->color+1;
- u8 *colors = (u8 *)dc->data;
+ OSDSetPalette(av7110, (u32 *)colors, dc->color, dc->x0);
+ else {
+ int i;
+ len = dc->x0-dc->color+1;
for (i = 0; i<len; i++)
OSDSetColor(av7110, dc->color + i,
colors[i * 4], colors[i * 4 + 1],
colors[i * 4 + 2], colors[i * 4 + 3]);
- }
+ }
+ kfree(colors);
return 0;
}
case OSD_SetTrans:
