|
Bugzilla – Full Text Bug Listing |
| Summary: | VUL-0: CVE-2004-0415: kernel: /proc info leak | ||
|---|---|---|---|
| Product: | [Novell Products] SUSE Security Incidents | Reporter: | Roman Drahtmueller <draht> |
| Component: | Incidents | Assignee: | Thomas Biege <thomas> |
| Status: | VERIFIED FIXED | QA Contact: | Security Team bot <security-team> |
| Severity: | Major | ||
| Priority: | P3 - Medium | CC: | afx, krahmer, security-team |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | CVE-2004-0415: CVSS v2 Base Score: 2.1 (AV:L/AC:L/Au:N/C:P/I:N/A:N) | ||
| Found By: | Other | Services Priority: | |
| Business Priority: | Blocker: | --- | |
| Marketing QA Status: | --- | IT Deployment: | --- |
| Attachments: |
vendor-sec discussion
forwarded by Alan Cox, work from Al Viro, needs review. from Solar designer via vendor-sec. preliminary, from Al Viro, against 2.4 kernels. |
||
|
Description
Sebastian Krahmer
2004-05-24 20:28:45 UTC
<!-- SBZ_reproduce --> ... CAN-2004-0415 CDR: Tuesday, June 29th, 16:00 MEST Do we have fixes for this problem? Not so far. But people start working on it... I am willing to accept a (tested) patch last minute before Goldmaster for this. My Goldmaster is June 30th, so this would work. But keep in mind that we give our partners our current kernels, so this should be checked in last minute (June 28th/28th). I'm currently in the process of preparing the 2.4 based update kernels. Since release date is June, 29th, I really urgently need the fix. And of course we also need it for SLES9 GA... fixing this issue seems not to be easy... (see vendor-sec@) Thomas, can you put the vendor-sec discussion somewhere where I can read it? Thanks? Created attachment 21716 [details]
vendor-sec discussion
Created attachment 21755 [details]
forwarded by Alan Cox, work from Al Viro, needs review.
From: Solar Designer <solar@openwall.com> To: vendor-sec@lst.de Date: Tue, 29 Jun 2004 00:18:33 +0400 Subject: [vendor-sec] workaround for the f_pos races Parts/Attachments: 1 Shown 55 lines Text 2 OK 139 lines Text ---------------------------------------- Hi, The patch Alan has posted looks good, but it's clear that in a patch this large there must be bugs. And it probably still misses some broken read/write routines. I'd like to propose an alternative approach which, when implemented cleaner than in the attached patch, might be more appropriate for preliminary vendor updates which are to be released in a matter of days (if not hours). I went ahead and introduced a mutex into struct file and I'm using the mutex to serialize all seek/read/write activity on a given fd. I've verified that my patch defeats Paul's mtrr exploit. It still thinks it wins the race, but the resulting kmem dump does not contain anything but the usual /proc/mtrr data: david!sources:/tmp$ ./isec-mtrr-ppos /usr/lib/librpm-4.2.so [+] mmaped uncached file at 0x25f000 - 0x3eb000 [+] mmaped kernel data file at 0x40000000 [+] Race won! [+] READ 0 bytes in 13946 usec david!sources:/tmp$ strings kmem.dat reg00: base=0x00000000 ( 0MB), size= 512MB: write-back, count=1 reg01: base=0x20000000 ( 512MB), size= 256MB: write-back, count=1 david!sources:/tmp$ Known disadvantages of this approach: - it breaks binary compatibility with old kernel modules (although in practice old modules will just work most of the time, -- mine do), but this happens between kernel releases anyway; - it might break some third-party kernel patches which allocate new struct file's, but won't know to initialize the mutex; - the serialization is more strict than was absolutely necessary to avoid the security issue (but is this a disadvantage?) Known problems with the patch in its present form: - it does not honor O_NONBLOCK when concurrent calls for the same fd are made by different threads of an application; - not all instances of ->{read,write} are covered, -- most of the remaining probably can't be triggered, -- for example, those in binary loaders shouldn't be triggerable since there's no fexec(), -- but there're some which need to be patched too (those which pertain to 32-bit syscall emulation on 64-bit platforms). Thoughts? -- /sd Created attachment 21757 [details]
from Solar designer via vendor-sec.
From: viro@parcelfarce.linux.theplanet.co.uk To: Solar Designer <solar@openwall.com> Cc: vendor-sec@lst.de Date: Mon, 28 Jun 2004 21:55:48 +0100 Subject: Re: [vendor-sec] workaround for the f_pos races On Tue, Jun 29, 2004 at 12:18:33AM +0400, Solar Designer wrote: > I went ahead and introduced a mutex into struct file and I'm using the > mutex to serialize all seek/read/write activity on a given fd. > Thoughts? You'll never get that merged upstream. It's a huge overkill and it _still_ doesn't solve the problems with broken wraparound checks, etc. Said problems are much more widespread than race-related "how to get negative offset" stuff - just check the patch. Moreover, for absolute majority of instances we don't need any locks at all, since these guys do not accept offsets past 4Gb, making updates of offset atomic for all practical purposes. _______________________________________________ From: Alan Cox <alan@lxorguk.ukuu.org.uk> To: Solar Designer <solar@openwall.com> Cc: vendor-sec@lst.de Date: Mon, 28 Jun 2004 21:04:23 +0100 Subject: Re: [vendor-sec] workaround for the f_pos races On Llu, 2004-06-28 at 21:18, Solar Designer wrote: > I went ahead and introduced a mutex into struct file and I'm using the > mutex to serialize all seek/read/write activity on a given fd. > - it does not honor O_NONBLOCK when concurrent calls for the same fd > are made by different threads of an application; You get deadlocks as well as killing system performance. I really do think there is a case for a set of helpers here for many of the users (and actually for a lot conversion to seq_* as recommended by Al would fix them). A lot of the drivers would benefit from a simple set of lseek/read/write helpers. I might try and make the case for that with Linus after the panic is over. _______________________________________________ From: Solar Designer <solar@openwall.com> To: viro@parcelfarce.linux.theplanet.co.uk Cc: vendor-sec@lst.de Date: Tue, 29 Jun 2004 01:13:28 +0400 Subject: Re: [vendor-sec] workaround for the f_pos races On Mon, Jun 28, 2004 at 09:55:48PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote: > On Tue, Jun 29, 2004 at 12:18:33AM +0400, Solar Designer wrote: > > I went ahead and introduced a mutex into struct file and I'm using the > > mutex to serialize all seek/read/write activity on a given fd. > > > Thoughts? > > You'll never get that merged upstream. I guess so. But it's a pity. Another interesting test case: 3< large_uncached_file_on_disk cat <&3 > f1_on_tmpfs & cat <&3 > f2_on_tmpfs & wait wc -c f?_on_tmpfs With an unpatched kernel, the sum of sizes of f1 and f2 will usually exceed the size of the original file, -- in my tests, by as much as 50%. This feels wrong. With my patch, the sum of sizes of f1 and f2 exactly matches the size of large_uncached_file_on_disk. > It's a huge overkill I disagree. I actually have a candidate application relying on the above property, -- which I once thought the kernel guaranteed... > and it _still_ > doesn't solve the problems with broken wraparound checks, etc. Yes, it's against the races only. As for broken wraparound checks, I think we may add wraparound checks at this same higher layer once we have the locks. Simply not call f_op->read() with arguments which would wraparound; instead, we can decrease the count for the call. I just don't want to rely on hundreds of individual implementations to be correct, -- I want to trap invalid input early on. > Said problems > are much more widespread than race-related "how to get negative offset" > stuff - just check the patch. OK, I trust you on this. > Moreover, for absolute majority of instances > we don't need any locks at all, since these guys do not accept offsets > past 4Gb, making updates of offset atomic for all practical purposes. You're assuming that the underlying f_ops are correct and don't load *ppos multiple times, -- which we know was not the case and will not be the case for many drivers which are yet to be developed by others. You're also only talking of the security aspect of it, -- but some of it is about correctness, not only security. See my example above. Introducing locks at the higher layer like I did eliminates races even for those poor f_ops which load *ppos multiple times. -- /sd I don't wrap this up into an attachment for speed. From: Alan Cox <alan@lxorguk.ukuu.org.uk> To: Solar Designer <solar@openwall.com> Cc: viro@parcelfarce.linux.theplanet.co.uk, vendor-sec@lst.de Date: Mon, 28 Jun 2004 21:28:46 +0100 Subject: Re: [vendor-sec] workaround for the f_pos races On Llu, 2004-06-28 at 22:13, Solar Designer wrote: > I disagree. I actually have a candidate application relying on the > above property, -- which I once thought the kernel guaranteed... I had that argument with Al yesterday and having been over SUSv2 the only thing it really has a lot to say on is O_APPEND, which does seem buggered on UDF. The wrap check has the problem we don't know _what_ the wrapping limit for the code is - 2Gb, 4Gb, 2^63 and 2^62 all appear, as does 2^16 bytes! - thus a helper function can do good stuff here but not top level code _______________________________________________ From: viro@parcelfarce.linux.theplanet.co.uk To: Solar Designer <solar@openwall.com> Cc: vendor-sec@lst.de Date: Mon, 28 Jun 2004 22:29:43 +0100 Subject: Re: [vendor-sec] workaround for the f_pos races On Tue, Jun 29, 2004 at 01:13:28AM +0400, Solar Designer wrote: > Another interesting test case: > > 3< large_uncached_file_on_disk > cat <&3 > f1_on_tmpfs & > cat <&3 > f2_on_tmpfs & > wait > wc -c f?_on_tmpfs > > With an unpatched kernel, the sum of sizes of f1 and f2 will usually > exceed the size of the original file, -- in my tests, by as much as 50%. > This feels wrong. > > With my patch, the sum of sizes of f1 and f2 exactly matches the size > of large_uncached_file_on_disk. > > > It's a huge overkill > > I disagree. I actually have a candidate application relying on the > above property, -- which I once thought the kernel guaranteed... ... until you went and checked it on some Unices and found that your assumption was more than just "not guaranteed", it was actually wrong on quite a few of them. Correct? Which makes any software relying on such assumption immediately broken. FWIW, anybody who mixes read() and lseek() on the same descriptor without logics in their code serializing those are also getting what they'd asked for - GIGO. It should not break the kernel, obviously, but kernel has no business being nice to inherently racy userland code. BTW, you do realize that lseek() in the middle of read() on the same descriptor will be lost? On regular files, on quite a few Unices, Linux included. IMNSHO, aside of security problem in the kernel it's a case of "Doctor, It Hurts When I Do It". _______________________________________________ From: Solar Designer <solar@openwall.com> To: Alan Cox <alan@lxorguk.ukuu.org.uk> Cc: vendor-sec@lst.de Date: Tue, 29 Jun 2004 01:29:27 +0400 Subject: Re: [vendor-sec] workaround for the f_pos races On Mon, Jun 28, 2004 at 09:04:23PM +0100, Alan Cox wrote: > On Llu, 2004-06-28 at 21:18, Solar Designer wrote: > > - it does not honor O_NONBLOCK when concurrent calls for the same fd > > are made by different threads of an application; Actually, upon a second thought I realize that O_NONBLOCK is a non-issue. I can explain why not if needed. > You get deadlocks Would you care to illustrate that by an example? I don't think I ever have a thread try to acquire a second f_pos_lock when already holding one (on a different file). I see no room for deadlocks here. But I might be missing something. > as well as killing system performance. I don't think this has a performance impact other than that associated with the few extra assembly instructions executed to acquire and release locks. By far most of the time the locks will be acquired immediately. The only way there can be a delay is when another thread is playing with the same fd in a manner which would change f_pos, -- that's already not normal given that Linux's behavior in that case is pretty much undefined as illustrated by the example in my previous e-mail. But even then it's just increased latency for the pair of operations on the fd. Yes, they won't execute in parallel on SMP (instead, a CPU will be available for use by other processes), -- but what use is parallel execution if the end result would be undefined? > I really > do think there is a case for a set of helpers here for many of the users > (and actually for a lot conversion to seq_* as recommended by Al would > fix them). > > A lot of the drivers would benefit from a simple set of lseek/read/write > helpers. I might try and make the case for that with Linus after the > panic is over. I wonder what kind of helpers you have in mind here. Care to provide an example? Thanks, -- /sd _______________________________________________ creating attachment with preliminary patch against 2.4 kernels. Created attachment 21767 [details]
preliminary, from Al Viro, against 2.4 kernels.
We have reviewed the complete patch (two people for each change), sent out observations upstream, and received an update from Alan Cox. I have found a few additional problems in the update. Building test kernels now; to me it's not entirely clear how we will go about testing this, yet. 4. august seems to be the new coordinated release date. Fix is in all our trees now except SLES7. Which is no longer maintained AFAIK. CRD: 3rd. August, 19:00 MEST packages approved (expect for s390*) <!-- SBZ_reopen -->Reopened by draht@suse.de at Tue Aug 10 18:51:13 2004, took initial reporter krahmer@suse.de to cc Re-opening, for possible missed /proc files in patch. Waiting for Helmut... Closing... Helmut has found that the file /proc/kcore doesn't have the fix. Cannot be considered security-relevant, though, for DAC checks on that file, and for the actual purpose of the file: To provide kernel memory. CVE-2004-0415: CVSS v2 Base Score: 2.1 (AV:L/AC:L/Au:N/C:P/I:N/A:N) |