Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 3 Jul 2011 12:24:39 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Vasiliy Kulikov <segoon@...nwall.com>
Cc: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, rientjes@...gle.com, wilsons@...rt.ca,
        security@...nel.org, kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH] proc: fix a race in do_io_accounting()

On Sun, Jul 3, 2011 at 3:39 AM, Vasiliy Kulikov <segoon@...nwall.com> wrote:
>
> The order of locking is similar to the one inside of
> ptrace_attach(): first goes cred_guard_mutex, then lock_task_sighand().

Hmm. mm_for_maps() uses mutex_lock_killable(), as does lock_trace.

And neither proc_pid_wchan() nor the fd following ones
(proc_pid_follow_link etc) use anything at all.

So I'm not sure. And do we really even care about the theoretical
race? Even if we do hit the race window and happen to get it just as a
process turns setuid, it would seem to be totally harmless (we're not
going to see any of the sensitive IO anyway).

So considering the lack of consistency in this area, I can't really
find it in myself to care very deeply.

That said, the lack of consistency itself is a bit annoying and
worrisome. Maybe some kind of helper like we do have for
"mm_for_maps()" would be a good idea - not because the potential races
are all that worrisome, but because inconsistencies in the kernel are
always signs of confusion, and confusion is always bad and a breeding
ground for potential bugs.

                           Linus

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.