Re: [CHECKER] Probable security holes in 2.6.5

[Chris Wright <chrisw@osdl.org>]

> [BUG]
> /home/kash/linux/linux-2.6.5/drivers/scsi/3w-xxxx.c:780:tw_chrdev_ioctl:
> ERROR:TAINT: 673:780:Passing unbounded user value "((1025 +
> (*tw_ioctl).data_buffer_length) - 1)" as arg 2 to function
> "copy_to_user", which uses it unsafely in model
> [SOURCE_MODEL=(lib,copy_from_user,user,taintscalar)]
> [SINK_MODEL=(lib,copy_to_user,user,trustingsink)]    [PATH= "error != 0"
> on line 674 is false] 
> 	}
> 
> 	tw_ioctl = (TW_New_Ioctl *)cpu_addr;
> 
> 	/* Now copy down the entire ioctl */
> Start --->
> 	error = copy_from_user(tw_ioctl, (void *)arg, data_buffer_length +
> sizeof(TW_New_Ioctl) - 1);
> 
> 	... DELETED 101 lines ...
> 
> 			retval = -ENOTTY;
> 			goto out2;
> 	}
> 
> 	/* Now copy the response to userspace */
> Error --->
> 	error = copy_to_user((void *)arg, tw_ioctl, sizeof(TW_New_Ioctl) +
> tw_ioctl->data_buffer_length - 1);
> 	if (error == 0)
> 		retval = 0;
> out2:

The data_buffer_length was validated once, however upon second copy
it's conceivable it could've been maliciously changed, so just use
validated one.  Patch below.

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

===== drivers/scsi/3w-xxxx.c 1.40 vs edited =====
--- 1.40/drivers/scsi/3w-xxxx.c	Tue Sep 23 08:00:30 2003
+++ edited/drivers/scsi/3w-xxxx.c	Fri Apr 16 14:48:24 2004
@@ -683,7 +683,7 @@
 			break;
 		case TW_OP_AEN_LISTEN:
 			dprintk(KERN_WARNING "3w-xxxx: tw_chrdev_ioctl(): caught TW_AEN_LISTEN.\n");
-			memset(tw_ioctl->data_buffer, 0, tw_ioctl->data_buffer_length);
+			memset(tw_ioctl->data_buffer, 0, data_buffer_length);
 
 			spin_lock_irqsave(tw_dev->host->host_lock, flags);
 			if (tw_dev->aen_head == tw_dev->aen_tail) {
@@ -777,7 +777,7 @@
 	}
 
 	/* Now copy the response to userspace */
-	error = copy_to_user((void *)arg, tw_ioctl, sizeof(TW_New_Ioctl) + tw_ioctl->data_buffer_length - 1);
+	error = copy_to_user((void *)arg, tw_ioctl, sizeof(TW_New_Ioctl) + data_buffer_length - 1);
 	if (error == 0)
 		retval = 0;
 out2: