From 98cd917c1ac348d5cd94beabecc3011dcaa0a0f2 Mon Sep 17 00:00:00 2001 From: Chris Wright Date: Mon, 19 Apr 2004 04:26:30 -0400 Subject: [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. --- drivers/net/wan/sdla.c | 1 + 1 file changed, 1 insertion(+) 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)) { -- cgit 1.2.3-korg