linux-kernel - Re: [PATCH V2 1/2] exec: don't wait for zombie threads with cred_guard

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Feb 2017 17:42:09 +1300
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Mika Penttilä <mika.penttila@...tfour.com>,
        Aleksa Sarai <asarai@...e.com>,
        Andy Lutomirski <luto@...capital.net>,
        Attila Fazekas <afazekas@...hat.com>,
        Jann Horn <jann@...jh.net>, Kees Cook <keescook@...omium.org>,
        Michal Hocko <mhocko@...nel.org>,
        Ulrich Obergfell <uobergfe@...hat.com>,
        linux-kernel@...r.kernel.org, David Howells <dhowells@...hat.com>
Subject: Re: [PATCH V2 1/2] exec: don't wait for zombie threads with cred_guard_mutex held

Oleg Nesterov <oleg@...hat.com> writes:

> de_thread() waits for other threads with ->cred_guard_mutex held and this
> is really bad because the time is not bounded, debugger can delay the exit
> and this lock has a lot of users (mostly abusers imo) in fs/proc and more.
> And this leads to deadlock if debugger tries to take the same mutex:

Oleg.  I looked at the history in proc of users of cred_guard_mutex
and the proc users are grabbing cred_guard_mutex for the proper
semantic reasons.  To avoid races with setuid exec that could result in
an information disclosure.

I do agree that a mutex is the wrong data structure for the job
cred_guard_mutex is performing.  The job of sorting ensuring debuggers
and proc processes see either the old version or the new version of the
task.

I need to play with the code but I suspect the best we can handle this
preventing both security issues and problems in the future is to create
a new task struct and populate it appropriate with the new data from
exec (at least in the case of setuid exec).

I am thinking of generalizing the case of a non-leader exec where we
have to assume the leaders pid.

I don't yet know what the performance implications would be but
that would clean the users up a lot.

On that score I believe we can incrementally approach that point and
only grab the cred_guard_mutex in exec if we are performing an exec that
changes the processes credentials.

Right now I don't think it introduces any new security information
disclosures but the moving of flush_signal_handlers outside of
cred_guard_mutex feels wrong.

Eric