Re: [CHECKER] potential dereference of user pointer errors

[Chris Wright]

* Junfeng Yang ([e-mail]) wrote:
> 
> [BUG] cosa_getidstr calls copy_to_user on arg, which implies arg is
> tainted.

True, arg is a pointer from userspace call to ioctl().

> /home/junfeng/linux-2.5.63/drivers/net/wan/cosa.c:1109:cosa_readmem:
> ERROR:TAINTED deferencing "d" tainted by [dist=2][called by
> cosa_ioctl_common:parm3 calling cosa_getidstr:parm1 calling
> copy_to_user:parm0]

Both cosa_readmem and cosa_download don't seem to do any validation of
the user supplied ptr at all before dereferncing it in get_user.  And
it'd make sense to use 'code' in cosa_reamdme (as in cosa_download)
instead of 'd->code'.  Jan, does this look OK?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/net/wan/cosa.c 1.17 vs edited =====
--- 1.17/drivers/net/wan/cosa.c	Mon Jan 13 17:11:59 2003
+++ edited/drivers/net/wan/cosa.c	Fri Mar 21 15:53:38 2003
@@ -1057,7 +1057,8 @@
 		return -EPERM;
 	}
 
-	if (get_user(addr, &(d->addr)) ||
+	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
+	    __get_user(addr, &(d->addr)) ||
 	    __get_user(len, &(d->len)) ||
 	    __get_user(code, &(d->code)))
 		return -EFAULT;
@@ -1098,7 +1099,8 @@
 		return -EPERM;
 	}
 
-	if (get_user(addr, &(d->addr)) ||
+	if (verify_area(VERIFY_READ, d, sizeof(*d)) ||
+	    __get_user(addr, &(d->addr)) ||
 	    __get_user(len, &(d->len)) ||
 	    __get_user(code, &(d->code)))
 		return -EFAULT;
@@ -1106,7 +1108,7 @@
 	/* If something fails, force the user to reset the card */
 	cosa->firmware_status &= ~COSA_FW_RESET;
 
-	if ((i=readmem(cosa, d->code, len, addr)) < 0) {
+	if ((i=readmem(cosa, code, len, addr)) < 0) {
 		printk(KERN_NOTICE "cosa%d: reading memory failed: %d\n",
 			cosa->num, i);
 		return -EIO;