Re: [CHECKER] Probable security holes in 2.6.5

[Chris Wright <chrisw@osdl.org>]

* Ken Ashcraft (ken@coverity.com) wrote:
> [BUG]
> /home/kash/linux/linux-2.6.5/drivers/net/e1000/e1000_ethtool.c:1494:e1000_ethtool_ioctl: ERROR:TAINT: 1487:1494:Passing unbounded user value "(regs).len" 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=] 
> 	}
> 	case ETHTOOL_GREGS: {
> 		struct ethtool_regs regs = {ETHTOOL_GREGS};
> 		uint32_t regs_buff[E1000_REGS_LEN];
> 
> Start --->
> 		if(copy_from_user(&regs, addr, sizeof(regs)))
> 			return -EFAULT;
> 		e1000_ethtool_gregs(adapter, &regs, regs_buff);
> 		if(copy_to_user(addr, &regs, sizeof(regs)))
> 			return -EFAULT;
> 
> 		addr += offsetof(struct ethtool_regs, data);
> Error --->
> 		if(copy_to_user(addr, regs_buff, regs.len))
> 			return -EFAULT;
> 
> 		return 0;

Looks like a bug.  Possible patch below zeros the buffer (since it's not
filled completely by e1000_ethtool_gregs()), and truncates len.

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

===== drivers/net/e1000/e1000_ethtool.c 1.42 vs edited =====
--- 1.42/drivers/net/e1000/e1000_ethtool.c	Fri Apr  9 16:39:34 2004
+++ edited/drivers/net/e1000/e1000_ethtool.c	Fri Apr 16 11:20:03 2004
@@ -1514,6 +1514,9 @@
 
 		if(copy_from_user(&regs, addr, sizeof(regs)))
 			return -EFAULT;
+		memset(regs_buff, 0, sizeof(regs_buff));
+		if (regs.len > E1000_REGS_LEN)
+			regs.len = E1000_REGS_LEN;
 		e1000_ethtool_gregs(adapter, &regs, regs_buff);
 		if(copy_to_user(addr, &regs, sizeof(regs)))
 			return -EFAULT;