updated-fbmem-patch.patch

updated-fbmem-patch.patch

[Andrea Arcangeli]

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mm4/broken-out/updated-fbmem-patch.patch

this uses get_user for the set_cmap operation too.

--- 2.6.5-aa3/drivers/video/fbmem.c.~1~	2004-04-04 08:09:23.000000000 +0200
+++ 2.6.5-aa3/drivers/video/fbmem.c	2004-04-21 03:11:05.878951424 +0200
@@ -1034,11 +1034,11 @@ fb_ioctl(struct inode *inode, struct fil
 	case FBIOPUTCMAP:
 		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
 			return -EFAULT;
-		return (fb_set_cmap(&cmap, 0, info));
+		return (fb_set_cmap(&cmap, 1, info));
 	case FBIOGETCMAP:
 		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
 			return -EFAULT;
-		return (fb_copy_cmap(&info->cmap, &cmap, 0));
+		return (fb_copy_cmap(&info->cmap, &cmap, 2));
 	case FBIOPAN_DISPLAY:
 		if (copy_from_user(&var, (void *) arg, sizeof(var)))
 			return -EFAULT;

this is the port to 2.4:

--- 2.4.23aa2/drivers/video/fbmem.c.~1~	2003-08-26 00:13:02.000000000 +0200
+++ 2.4.23aa2/drivers/video/fbmem.c	2004-04-21 03:13:34.545350696 +0200
@@ -511,11 +511,11 @@ fb_ioctl(struct inode *inode, struct fil
 	case FBIOPUTCMAP:
 		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
 			return -EFAULT;
-		return (fb->fb_set_cmap(&cmap, 0, PROC_CONSOLE(info), info));
+		return (fb->fb_set_cmap(&cmap, 1, PROC_CONSOLE(info), info));
 	case FBIOGETCMAP:
 		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
 			return -EFAULT;
-		return (fb->fb_get_cmap(&cmap, 0, PROC_CONSOLE(info), info));
+		return (fb->fb_get_cmap(&cmap, 1, PROC_CONSOLE(info), info));
 	case FBIOPAN_DISPLAY:
 		if (copy_from_user(&var, (void *) arg, sizeof(var)))
 			return -EFAULT;


(1 not a typo)

Let me know if you see something wrong, it's untested.

Re: updated-fbmem-patch.patch

[Chris Wright]

* Andrea Arcangeli (andrea@suse.de) wrote:
> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mm4/broken-out/updated-fbmem-patch.patch
> 
> this uses get_user for the set_cmap operation too.
> 
> --- 2.6.5-aa3/drivers/video/fbmem.c.~1~	2004-04-04 08:09:23.000000000 +0200
> +++ 2.6.5-aa3/drivers/video/fbmem.c	2004-04-21 03:11:05.878951424 +0200
> @@ -1034,11 +1034,11 @@ fb_ioctl(struct inode *inode, struct fil
>  	case FBIOPUTCMAP:
>  		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
>  			return -EFAULT;
> -		return (fb_set_cmap(&cmap, 0, info));
> +		return (fb_set_cmap(&cmap, 1, info));

0 is userspace, 1 is kernel space.  this change looks wrong.

Perhaps the change below so comment is in sync with code.

--- a/drivers/video/fbcmap.c	Fri Feb  6 00:30:15 2004
+++ b/drivers/video/fbcmap.c	Wed Apr 21 15:40:56 2004
@@ -207,7 +207,7 @@
 /**
  *	fb_set_cmap - set the colormap
  *	@cmap: frame buffer colormap structure
- *	@kspc: boolean, 0 copy local, 1 get_user() function
+ *	@kspc: boolean, 0 get_user() function , 1 copy local
  *	@info: frame buffer info structure
  *
  *	Sets the colormap @cmap for a screen of device @info.

Re: updated-fbmem-patch.patch

[Andrea Arcangeli]

On Wed, Apr 21, 2004 at 03:41:53PM -0700, Chris Wright wrote:
> * Andrea Arcangeli (andrea@suse.de) wrote:
> > http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mm4/broken-out/updated-fbmem-patch.patch
> > 
> > this uses get_user for the set_cmap operation too.
> > 
> > --- 2.6.5-aa3/drivers/video/fbmem.c.~1~	2004-04-04 08:09:23.000000000 +0200
> > +++ 2.6.5-aa3/drivers/video/fbmem.c	2004-04-21 03:11:05.878951424 +0200
> > @@ -1034,11 +1034,11 @@ fb_ioctl(struct inode *inode, struct fil
> >  	case FBIOPUTCMAP:
> >  		if (copy_from_user(&cmap, (void *) arg, sizeof(cmap)))
> >  			return -EFAULT;
> > -		return (fb_set_cmap(&cmap, 0, info));
> > +		return (fb_set_cmap(&cmap, 1, info));
> 
> 0 is userspace, 1 is kernel space.  this change looks wrong.
> 
> Perhaps the change below so comment is in sync with code.

yes, I was mislead by the comment, that's why you will never ever see a
single comments describing parameters in my code (except if the stuff
really isn't obvious and it cannot be trivially deduced by the code and
in turn it _deserves_ a fat comment, definitely not in this case).
There's no way I can trust comments anyways, I perfectly know I cannot
stop after reading a comment, I went ahead and I checked if the comment
was right but it was too late and I was already biased by what I read in
the comment so I didn't notice the comment was wrong. This isn't a good
excuse, it's still my bad mistake, shame on me, but it certainly gives
more strenght to my no-comment-for-obvious-parameters policy. I don't
care if you cannot do a pdf of the whole kernel anymore, that pdf is
likely buggy anyways and you'd better not attempt to read it in the
first place.

Arjan and Andrew also notified me privately. thanks to all for noticing
and sorry for the stupid mistake of being influenced by a worthless
comment.