Commit bf31652c authored by Chris Wright's avatar Chris Wright Committed by Linus Torvalds

[PATCH] e1000: fix probable security hole

* 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.
parent 27de7466
...@@ -1514,6 +1514,9 @@ e1000_ethtool_ioctl(struct net_device *netdev, struct ifreq *ifr) ...@@ -1514,6 +1514,9 @@ e1000_ethtool_ioctl(struct net_device *netdev, struct ifreq *ifr)
if(copy_from_user(&regs, addr, sizeof(regs))) if(copy_from_user(&regs, addr, sizeof(regs)))
return -EFAULT; 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); e1000_ethtool_gregs(adapter, &regs, regs_buff);
if(copy_to_user(addr, &regs, sizeof(regs))) if(copy_to_user(addr, &regs, sizeof(regs)))
return -EFAULT; return -EFAULT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment