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

[Pekka Pietikainen <pp@ee.oulu.fi>]

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: