Re: [patch] Avoid using hardcoded values in kernel/sys.c

From: Linus Torvalds
Date: Mon Oct 17 2011 - 21:20:41 EST


On Mon, Oct 17, 2011 at 5:55 PM, David Rientjes <rientjes@xxxxxxxxxx> wrote:
> The release field of struct new_utsname may always change, so avoid
> hardcoded values when setting up a buffer to copy to it.

This patch is totally bogus.

> +static int override_release(char __user *release)
>  {
> +       const int len = sizeof(release);

You just wrote "len = 4" (or 8, for 64-bit architectures) in a very unclear way.

IOW, "len" is now the size of the *pointer*. Which is wrong.

*And* you then re-introduced the variable-length array, which not only
triggered the gcc bug, but which is a BAD IDEA TO BEGIN WITH.

Variably sized arrays generate worse code, and can have subtle
security issues that aren't obvious (like cause our tools that check
for overly big stack usage to quietly fail).

Don't use variable-sized arrays on stack. They are never a good idea
in kernel space. The fact that we do have other code that does use
them too is not an excuse.

But using them in this kind of horribly broken manner with the wrong
length is just *really* bad.

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