diff options
author | Chris Wright <chrisw@osdl.org> | 2004-04-19 04:26:30 -0400 |
---|---|---|
committer | Jeff Garzik <jgarzik@redhat.com> | 2004-04-19 04:26:30 -0400 |
commit | 98cd917c1ac348d5cd94beabecc3011dcaa0a0f2 (patch) | |
tree | 4e4783586735cb66290036bde58c6a78b1d0166d | |
parent | 3ce12aab463fc05cc70f3b844a06c7857d920781 (diff) |
[PATCH] wan sdla: fix probable security hole
> [BUG] minor
> /home/kash/linux/linux-2.6.5/drivers/net/wan/sdla.c:1206:sdla_xfer:
> ERROR:TAINT: 1201:1206:Passing unbounded user value "(mem).len" as arg 0
> to function "kmalloc", which uses it unsafely in model
> [SOURCE_MODEL=(lib,copy_from_user,user,taintscalar)]
> [SINK_MODEL=(lib,kmalloc,user,trustingsink)] [MINOR] [PATH=] [Also
> used at, line 1219 in argument 0 to function "kmalloc"]
> static int sdla_xfer(struct net_device *dev, struct sdla_mem *info, int
> read)
> {
> struct sdla_mem mem;
> char *temp;
>
> Start --->
> if(copy_from_user(&mem, info, sizeof(mem)))
> return -EFAULT;
>
> if (read)
> {
> Error --->
> temp = kmalloc(mem.len, GFP_KERNEL);
> if (!temp)
> return(-ENOMEM);
> sdla_read(dev, mem.addr, temp, mem.len);
Hrm, I believe you could use this to read 128k of kernel memory.
sdla_read() takes len as a short, whereas mem.len is an int. So,
if mem.len == 0x20000, the allocation could still succeed. When cast
to short, len will be 0x0, causing the read loop to copy nothing into
the buffer. At least it's protected by a capable() check. I don't
know what proper upper bound is for this hardware, or how much it's
used/cared about. Simple memset() is trivial fix.
-rw-r--r-- | drivers/net/wan/sdla.c | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/drivers/net/wan/sdla.c b/drivers/net/wan/sdla.c index 3a8432062154d..0abba0f129063 100644 --- a/drivers/net/wan/sdla.c +++ b/drivers/net/wan/sdla.c @@ -1206,6 +1206,7 @@ static int sdla_xfer(struct net_device *dev, struct sdla_mem *info, int read) temp = kmalloc(mem.len, GFP_KERNEL); if (!temp) return(-ENOMEM); + memset(temp, 0, mem.len); sdla_read(dev, mem.addr, temp, mem.len); if(copy_to_user(mem.data, temp, mem.len)) { |