Thread (20 messages) 20 messages, 6 authors, 2010-11-01

Re: [patch v3] fix stack overflow in pktgen_if_write()

From: Nelson Elhage <hidden>
Date: 2010-10-28 15:22:25

You've got a leak if copy_user fails.

While testing this, I realized that printk() won't print more than 1k in a
single call, anyways, so I've sent along a patch that just copies up to 1k onto
the stack, which should prevent the overflow without changing behavior or
needing a heap allocation.

- Nelson

On Thu, Oct 28, 2010 at 08:05:29AM +0200, Dan Carpenter wrote:
quoted hunk ↗ jump to hunk
Nelson Elhage says he was able to oops both amd64 and i386 test 
machines with 8k writes to the pktgen file.  Let's just allocate the
buffer on the heap instead of on the stack.

This can only be triggered by root so there are no security issues here.

Reported-by: Nelson Elhage <redacted>
Signed-off-by: Dan Carpenter <redacted>
---
v3:  just use kmalloc()
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 2c0df0f..c8d3620 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -887,12 +887,17 @@ static ssize_t pktgen_if_write(struct file *file,
 	i += len;
 
 	if (debug) {
-		char tb[count + 1];
+		char *tb;
+
+		tb = kmalloc(count + 1, GFP_KERNEL);
+		if (!tb)
+			return -ENOMEM;
 		if (copy_from_user(tb, user_buffer, count))
 			return -EFAULT;
 		tb[count] = 0;
 		printk(KERN_DEBUG "pktgen: %s,%lu  buffer -:%s:-\n", name,
 		       (unsigned long)count, tb);
+		kfree(tb);
 	}
 
 	if (!strcmp(name, "min_pkt_size")) {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help