Re: Fw: Re: oops in choose_configuration()

[Linus Torvalds <torvalds@osdl.org>]

Andrew,
 I'm worried about the fact that kmalloc() doesn't get redlining.

The lifetime rules for the pipe_info thing and the bprm seems totally 
obvious, and we'd get slab errors if somebody was doing something strange 
there anyway.

So I'd be more inclined to blame a buffer overflow on a kmalloc, and the 
obvious target is the "add_uevent_var()" thing, since all/many of the 
corruptions seem to come from uevent environment variable strings.

Because kmalloc() doesn't do redlining, we'd never see an overflow as an 
error, and it would just overwrite the next block. Now, they are in 
different slab blocks (the uevent strign allocation is a 2048-byte 
allocation), but maybe it flows over into the next page entirely..

Now, it all uses "vnsprintf()" which _should_ be safe, but that in turn 
uses pointer comparisons, so maybe gcc screws that up. Who knows. Gcc has 
been known to use signed comparisons on pointers and other brokenness. And 
we could just have screwed something up (not updating "len" when we update 
the buffer start etc etc)

Anyway, this trivial patch will check for buffer length consistency and 
overflow by just putting a magic value at the end of the buffer, and 
checking it. Maybe.

I don't see anything wrong there, and booting with this patch doesn't 
trigger anything for me, but it's simple enough to be worth checking out.

			Linus
---
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 086a0c6..366214a 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -51,6 +51,9 @@ static char *action_to_string(enum kobje
 	}
 }
 
+#define UEV_MAGIC (0xc0edbabeu)
+#define uev_magic(buffer, len) ((unsigned int *) ((len) + (void *)(buffer)))[-1]
+
 /**
  * kobject_uevent - notify userspace by ending an uevent
  *
@@ -107,6 +110,8 @@ void kobject_uevent(struct kobject *kobj
 	if (!buffer)
 		goto exit;
 
+	uev_magic(buffer, BUFFER_SIZE) = UEV_MAGIC;
+
 	/* complete object path */
 	devpath = kobject_get_path(kobj, GFP_KERNEL);
 	if (!devpath)
@@ -223,6 +228,10 @@ int add_uevent_var(char **envp, int num_
 {
 	va_list args;
 
+	BUG_ON(buffer_size < 4);
+	BUG_ON(buffer_size > 2048);
+	BUG_ON(uev_magic(buffer, buffer_size) != UEV_MAGIC);
+
 	/*
 	 * We check against num_envp - 1 to make sure there is at
 	 * least one slot left after we return, since kobject_uevent()

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/