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]

Message-ID: <20170221175354.GA31436@redhat.com>
Date:   Tue, 21 Feb 2017 18:53:55 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.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
Subject: Re: [PATCH V2 1/2] exec: don't wait for zombie threads with
        cred_guard_mutex held

On 02/21, Eric W. Biederman wrote:
>
> Today cred_guard_mutex is part of making exec appear to be an atomic
> operation to ptrace and and proc.  To make exec appear to be atomic
> we do need to take the mutex at the beginning and release it at the end
> of exec.
>
> The semantics of exec appear atomic to ptrace_attach and to proc readers
> are necessary to ensure we use the proper process credentials in the
> event of a suid exec.

This is clear. My point is that imo a) it is over-used in fs/proc and b)
the scope of this mutex if execve is too huge. I see absolutely no reason
to do copy_strings() with this mutex held, for example. And note that
copy_strings() can use a lot of memory/time, it can trigger oom,swapping,
etc.

But let me repeat, this is a bit off-topic right now, this patch doesn't
change anything in this respect, afaics.

> I believe making cred_guard_mutex per task is an option.  Reducing the
> scope of cred_guard_mutex concerns me.  There appear to be some fields
> like sighand that we currently expose in proc

please see another email, collect_sigign_sigcatch() is called without this
mutex.

> Do you know if we can make cred_guard_mutex a per-task lock again?

I think we can, but this needs some (afaics simple) changes too.

But for what? Note that the problem fixed by this series won't go away
if we do this.

So what do you think about this series?

Oleg.