lkml.org 
[lkml]   [2008]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [stable] Linux 2.6.25.10 (resume)
Al Viro escreveu:
> On Sat, Jul 19, 2008 at 03:13:43PM -0700, Greg KH wrote:
>
>
>> I disagree with this and feel that our current policy of fixing bugs and
>> releasing full code is pretty much the same thing as we are doing today,
>> although I can understand the confusion. How about this rewording of
>> the sentance instead:
>>
>> We prefer to fix and provide an update for the bug as soon as
>> possible.
>>
>> So a simple 1 line change should be enough to stem this kind of argument
>> in the future, right?
>>
>
> Not quite... OK, here's a story that might serve as a model
> of all that crap - it certainly runs afoul of a bunch of arguments on
> all sides of that.
>
Ual, crap? :-)

> We all know that POSIX locks suck by design, in particular where it
> deals with close(2) semantics. "$FOO is associated with process P having
> a descriptor refering to opened file F, $FOO disappears when any of such
> descriptors get removed" is bloody inconvenient in a lot of respects. It
> also turns out to invite very similar kind of wrong assumptions in all
> implementation that have to deal with descriptor tables being possibly
> shared. So far the victims include:
> * FreeBSD POSIX locks; used to be vulnerable, fixed.
> * OpenBSD POSIX locks; vulnerable.
> * Linux POSIX locks and dnotify entries; used to be vulnerable, fixed.
> Plan9 happily avoids having these turds in the first place and IIRC NetBSD
> simply doesn't have means for sharing descriptor tables. Should such means
> appear it would be vulnerable as well. Dnotify is Linux-only, er, entity
> (as in "non sunt multiplicandam"). I haven't looked at Solaris and I couldn't
> care less about GNU Turd.
>
> In all cases vulnerablities are local, with impact ranging from
> user-triggered panic to rather unpleasant privelege escalations (e.g.
> "any user can send an arbitrary signal to arbitrary process" in case of
> dnotify, etc.)
>
> The nature of mistaken assumption is exactly the same in all cases.
> An object is associated with vnode/dentry/inode/whatnot and since it's
> destroyed on any close(), it is assumed that we shall be guaranteed that
> such objects will not be able to outlive the opened file they are associated
> with or the process creating them. It leads to the following nastiness:
> A and B share descriptor table.
> A: fcntl(fd, ...) trying to create such object; it resolves descriptor to
> opened file, pins it down for the duration of operation and blocks
> somewhere in course of creating the object.
> B: close(fd) evicts opened file from descriptor table. It finds no objects
> to be destroyed.
> A: completes creation of object, associates it with filesystem object and
> releases the temporary hold it had on opened file.
>
> At that point we have an obvious leak and slightly less obvious attack vector.
> If no other descriptors in the descriptor table of A and B used to refer to
> the same file, the object will not be destroyed since there will be nothing
> that could decide to destroy it. Unfortunately, it's worse than just a leak.
> These objects are supposed to be destroyed before the end of life of opened
> file. As the result, nobody bothers to have them affect refcounts on the
> file/vnode/dentry/inode/whatever. That's perfectly fine - the proper fix is
> to have fcntl() verify that descriptor still resolves to the same file before
> completing its work and destroy the object if it doesn't. You don't need to
> play with refcounts for that. However, without that fix we have a leak that
> leads to more than just an undead object - it's an undead object containing
> references to filesystem object that might be reused and to task_struct/proc/
> whatever you call it, again with the possibility of reuse.
>
> Getting from that point to the attack is a matter of simple (really simple)
> RTFS. Details obviously depend on what the kernel in question is doing to
> these objects, but with that kind of broken assertions it's really not hard
> to come up with exploitable holes.
>
> Now, let us look at the history:
> * POSIX locks support predates shared descriptor tables; the holes
> in question opened as soon as clone(2)/rfork(2) had been merged into a kernel
> and grown support for shared descriptor tables. For Linux it's 1.3.22 (Sep
> 1995), for OpenBSD it's a bit before 2.0 (Jan 1996) for FreeBSD - 2.2 (Feb
> 1996, from OpenBSD).
> * In 2002 dnotify had grown the same semantics (Linux-only thing,
> 2.5.15, soon backported to 2.4.19). Same kind of race.
> * In 2003 FreeBSD folks had found and fixed their instance of that bug;
> commit message:
> "Avoid file lock leakage when linuxthreads port or rfork is used:
> - Mark the process leader as having an advisory lock
> - Check if process leader is marked as having advisory lock when
> closing file
> - Check that file is still open after lock has been obtained
> - Don't allow file descriptor table sharing between processes
> with different leaders"
> "Check that file is still open" bit refers to that race. I have no idea
> whether they'd realized that what they'd closed had been locally exploitable
> or not.
> * In 2005 Peter Staubach had noticed the Linux analog of that sucker.
> The fix had been along the same lines as FreeBSD one, but in case of Linux
> we had extra fun with SMP ordering. Peter had missed that and his patch
> left a hard to hit remnant of the race. His commit message is rather long;
> it starts with
> [PATCH] stale POSIX lock handling
>
> I believe that there is a problem with the handling of POSIX locks, which
> the attached patch should address.
>
> The problem appears to be a race between fcntl(2) and close(2). A
> multithreaded application could close a file descriptor at the same time as
> it is trying to acquire a lock using the same file descriptor. I would
> suggest that that multithreaded application is not providing the proper
> synchronization for itself, but the OS should still behave correctly.
> ...
> I'm 100% sure that in this case the vulnerability had _not_ been realized.
> Bogus behaviour had been noticed and (mostly) fixed, but implications had
> been missed, along with the fact that the same scenario had played out in
> dnotify.
>
That's common... We know that will occur from time to time and we are
not discussing about that... Just about already known security issues
being silently fixed without any mention.
> * This April I'd caught dnotify hole during code audit. The fix
> had been trivial enough and seeing that impact had been fairly nasty (any
> user could send any signal to any process, among other things) I'd decided
> to play along with "proper mechanisms". Meaning vendor-sec. _Bad_ error
> in judgement; the damn thing had not improved since I'd unsubscribed from
> that abortion. A trivial patch, obviously local to one function and obviously
> not modifying behaviour other than in case when existing tree would've screwed
> itself. Not affecting any headers. Not affecting any data structures.
> _Obviously_ not affecting any 3rd-party code - not even on binary level.
> IOW, as safe as it ever gets.
> Alas. The usual shite had played out and we had a *MONTH-LONG*
> embargo. I would like to use this opportunity to offer a whole-hearted
> "fuck you" to that lovely practice and to vendor-sec in general.
>
huahuahuahuahuhuahuahu, tks for share your felling about that... Maybe
the wrong people are dealing with that... What do you think?
> * Very soon after writing the first version of a fix I started
> wondering if POSIX locks had the same hole - the same kind of semantics
> had invited the same kind of race there (eviction of dnotify entries and
> eviction of POSIX locks are called in the same place in close(2)). Current
> tree appeared to be OK, checking the history had shown Peter's patch.
> A bit after that I'd noticed insufficient locking in dnotify patch, fixed
> that. Checking for similar problems in POSIX locks counterpart of that crap
> had found the SMP ordering bug that remained there.
> * 2.6 -> 2.4 backport had uncovered another interesting thing -
> Peter's patch did _not_ make it to 2.4 3 years ago.
> * Checking OpenBSD (only now - I didn't get around to that back in
> April) shows that the same hole is alive and well there.
>
> Note that
> * OpenBSD folks hadn't picked the fix from FreeBSD ones, even though
> the FreeBSD version had been derived from OpenBSD one. Why? At a guess, the
> commit message had been less than noticable. Feel free to toss yourself off
> screaming "coverup" if you are so inclined; I don't swing that way...
>
I ever used this word... I know others did anyway.
> * Initial Linux fix _definitely_ had missed security implications
> *and* realization that somebody else might suffer from the same problem.
> FVO "somebody" including Linux itself, not to mention *BSD.
>
That's the main problem of 'hidden' security bugs... Hidden here must be
understood as: normal bugs = security bugs
> * Even when the problem had been realized for what it had been in
> Linux, *BSD potential issues hadn't registered. Again, the same wunch
> of bankers is welcome to scream "coverup", but in this case we even have
> the bleeding CVEs.
> * CVEs or no CVEs, OpenBSD folks hadn't picked that one.
>
Yeah, they don't pay much attention to other projects I they should...
That's why I always talk about 'copy and paste security bugs' as a real
problem in open-source projects... Because the original code may be
fixed and the 'copied' one not ;)
> * Going to vendor-sec is a mistake I won't repeat any time soon and
> I would strongly recommend everybody else to stay the hell away from that
> morass. It creates inexcusable delays, bounds you to confidentiality and,
>
Interesting... every people involved in this discussion told us to
change our behaviour and try to improve things instead of be against
it... Why not change people involved in the vendor-sec in some way? I
have no idea who are there, but I'm sure it can be improved since I
already worked with many vendors to coordinate vuln-disclousure and had
no problems.

I'm sure the vendors involved are caring about 'marketing' and things
like that, that's why there is a delay... They need to learn from others
mistakes, like Microsoft. The company now really knows how to deal with
security problems.
> let's face it, happens to be the prime infiltration target for zero-day
> exploit traders. In _this_ case exploit had been local-only. Anything more
> interesting...
>
That's true. Mainly because it takes longer to have a fix... I agree
with the fix asap idea, just don't agree with 'change the message in the
patch to not apper to be a security bug been fixed'.
> So where does that leave us? Bugger if I know... FWIW, I would rather see
> implications thought about *and* mentioned in the changelogs. OTOH, the
> above shows the real-world cases when breakage hadn't even been realized to
> be security-significant.
We ever said it's different otherwise...
> Obviously broken behaviour (leak, for example)
> gets spotted and fixed. Fix looks obviously sane, bug it deals with -
> obviously real and worth fixing, so into a tree it goes... IOW, one _can't_
> rely on having patches that close security holes marked as such.
Sometimes no one will figure out that patch closed a security issue...
We are talking in this thread about patches that are well-known to fix
security holes... As the bugzilla explicitly says about a security
problem and in the commit nothing mention that.
> For that
> the authors have to notice that themselves in the first place. OTTH, nothing
> is going to convince the target audience of grsec, er, gentlemen that we are
> not a massive conspiracy anyway, the tinfoil brigade being what it is...
>

There is no target audience involved here... There is no 'marketing' or
'media circus'... It's just a try to improve things.


cya,


Rodrigo (BSDaemon).


PS: Just my personal thoughts, not related to the company that I work for.


\
 
 \ /
  Last update: 2008-07-22 03:13    [W:0.246 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site