Arch Linux

Please read this before reporting a bug:
https://wiki.archlinux.org/title/Bug_reporting_guidelines

Do NOT report bugs when a package is just outdated, or it is in the AUR. Use the 'flag out of date' link on the package page, or the Mailing List.

REPEAT: Do NOT report bugs for outdated packages!
Tasklist

FS#15556 - [kernel26] CVE-2009-1987 null pointer dereference vulnerability in tun_chr_poll()

Attached to Project: Arch Linux
Opened by Kerin Millar (kerframil) - Saturday, 18 July 2009, 02:59 GMT
Last edited by Roman Kyrylych (Romashka) - Wednesday, 22 July 2009, 18:21 GMT
Task Type Bug Report
Category Security
Status Closed
Assigned To Tobias Powalowski (tpowa)
Architecture All
Severity High
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This task depends upon

Closed by  Roman Kyrylych (Romashka)
Wednesday, 22 July 2009, 18:21 GMT
Reason for closing:  Fixed
Additional comments about closing:  Linux-2.6.30.2 is now in [core]
Comment by Gerardo Exequiel Pozzi (djgera) - Saturday, 18 July 2009, 03:23 GMT
a bug introduced by gcc optimization :P
Comment by Glenn Matthys (RedShift) - Saturday, 18 July 2009, 07:57 GMT
This probably only applies when you have the tun module loaded.
Comment by Roman Kyrylych (Romashka) - Saturday, 18 July 2009, 17:43 GMT
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).
Comment by Corrado Primier (bardo) - Saturday, 18 July 2009, 17:51 GMT
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.
Comment by Kerin Millar (kerframil) - Sunday, 19 July 2009, 01:26 GMT
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/
Comment by Roman Kyrylych (Romashka) - Sunday, 19 July 2009, 09:12 GMT
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.
Comment by Corrado Primier (bardo) - Sunday, 19 July 2009, 09:44 GMT
@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).
Comment by Kerin Millar (kerframil) - Sunday, 19 July 2009, 16:35 GMT
@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/
Comment by Gerardo Exequiel Pozzi (djgera) - Monday, 20 July 2009, 04:55 GMT
2.6.30.2 is released fixing the code, and also adding -fno-delete-null-pointer-checks to CFLAGS :)

Loading...