Arch Linux

a bug introduced by gcc optimization :P

This probably only applies when you have the tun module loaded.

lowered the severity since it's not remotely exploitable and Brad Spengler's exploit does not work on most people machines actually (I've seen lots of users of one Linux forum tried it without success).

I confirm this didn't work for me. It doesn't compile on x86_64 (seems to expose a gcc bug ;), and on i686 can't get root. Anyway, the vulnerability is still dangerous. As exploit.c points out the solution is disabling that particular gcc optimization for the kernel, so adding -fno-delete-null-pointer-checks should insert again the null check in the binary and fix the issue.

Hang on, a bug is a bug. There's no sense in deferencing a pointer before checking that it is NULL. Yes, adding -fno-delete-null-pointer-checks would have mitigated the *impact* of the bug but I would call that a sensible precautionary measure rather than a solution, as the solution to coding errors is to fix the code - commit 3c8a9c63d5fd738c261bd0ceece04d9c8357ca13 in this case. However, upstream have learned their lesson and added a patch to enable -fno-delete-null-pointer-checks to the stable queue:

http://patchwork.kernel.org/patch/36060/

Yes, noone says it's not a bug, just not that critical. I'm just trying to make sure that severities of reports are more or less realistic.
"Critical" is for extreme cases, like something that gets an HDD with all your data burned, or remotely take over your system.

@kerframil: in my understanding that's not the bug we're talking about. The commit you linked in the report fixes a NULL dereferentiation (the !tun check), but it is this fix that makes the new exploit possible. That's why the vulnerability is interesting and new: because when compiling without -fno-delete-null-pointer-checks, gcc removes the NULL check, assuming that anyway dereferencing to NULL would cause an exception and be handled accordingly. But then, Spengler notes that it /is/ possible to map memory at the NULL address, so this is why the check should always be there in the kernel (and other - suid - binaries, methinks).

@bardo:

> That's why the vulnerability is interesting and new: because when compiling without
> -fno-delete-null-pointer-checks, gcc removes the NULL check, assuming that anyway
> dereferencing to NULL would cause an exception and be handled accordingly

This is false. The reason the check is removed is because the deferencing has already occurred at that point.

As it stands, sk is initialised to tun->sk and only thereafter is tun compared to NULL. The fact that the null pointer check occurs after the assignment in the original code - which involves dereferencing the pointer - is what makes gcc think it's OK to optimise away said check (as it makes the arguably reasonable assumption that once it has been dereferenced it is not NULL and so further checks are unnecessary). As I have already stated, this bad code was introduced in commit 33dccbb050bbe35b88ca8cf1228dcf3e4d4b3554. Look, it's even confirmed by the cheddar_bay code itself (the comment in exploit.c):

"The commit that introduced the vulnerability (Feb 6th)
http://mirror.celinuxforum.org/gitstat/commit-detail.php?commit=33dccbb050bbe35b88ca8cf1228dcf3e4d4b3554
Though it was committed before the release of the 2.6.29 kernel, it
did not (thankfully) make it into the 2.6.29 kernel. It first
appeared in 2.6.30."

Irrespective of how gcc behaves, if the pointer is potentially NULL, the check should occur before it's dereferenced; that it does not is a bona fide bug and _this_ is what I am talking about. And so, commit 3c8a9c63d5fd738c261bd0ceece04d9c8357ca13 ensures the the pointer is not dereferenced until after the check thus resolving the issue correctly:

36 - struct sock *sk = tun->sk;
37 + struct sock *sk;
38 unsigned int mask = 0;
39
40 if (!tun)
41 return POLLERR;
42
43 + sk = tun->sk;

The previous behaviour is so obviously wrong, I don't understand how there can be any confusion regarding it.

The aforementioned patch has been queued for 2.6.30.2. Meanwhile, 2.6.30 and 2.6.30.1 are potentially vulnerable. If you want to be certain that your userbase is protected prior to the arrival of 2.6.30.2, the patch should be applied.

PS: Jonathan Corbet touches upon a particularly interesting facet of the matter here: http://lwn.net/Articles/341959/

2.6.30.2 is released fixing the code, and also adding -fno-delete-null-pointer-checks to CFLAGS :)