linux-kernel - Re: [PATCH v2 5/5] exec: Add a exec_update_mutex to replace 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: <871rq12vxu.fsf@x220.int.ebiederm.org>
Date:   Mon, 09 Mar 2020 14:02:37 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Bernd Edlinger <bernd.edlinger@...mail.de>
Cc:     Christian Brauner <christian.brauner@...ntu.com>,
        Kees Cook <keescook@...omium.org>,
        Jann Horn <jannh@...gle.com>, Jonathan Corbet <corbet@....net>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Oleg Nesterov <oleg@...hat.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Andrei Vagin <avagin@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        "Peter Zijlstra \(Intel\)" <peterz@...radead.org>,
        Yuyang Du <duyuyang@...il.com>,
        David Hildenbrand <david@...hat.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Anshuman Khandual <anshuman.khandual@....com>,
        David Howells <dhowells@...hat.com>,
        James Morris <jamorris@...ux.microsoft.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shakeel Butt <shakeelb@...gle.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Christian Kellner <christian@...lner.me>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Aleksa Sarai <cyphar@...har.com>,
        "Dmitry V. Levin" <ldv@...linux.org>,
        "linux-doc\@vger.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-fsdevel\@vger.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-mm\@kvack.org" <linux-mm@...ck.org>,
        "stable\@vger.kernel.org" <stable@...r.kernel.org>,
        "linux-api\@vger.kernel.org" <linux-api@...r.kernel.org>
Subject: Re: [PATCH v2 5/5] exec: Add a exec_update_mutex to replace cred_guard_mutex

Bernd Edlinger <bernd.edlinger@...mail.de> writes:

> On 3/9/20 7:36 PM, Eric W. Biederman wrote:
>> 
>> 
>> Does that sound better?
>> 
>
> almost done.

I think this text is finally clean.

    exec: Add exec_update_mutex to replace cred_guard_mutex
    
    The cred_guard_mutex is problematic as it is held over possibly
    indefinite waits for userspace.  The possilbe indefinite waits for
    userspace that I have identified are: The cred_guard_mutex is held in
    PTRACE_EVENT_EXIT waiting for the tracer.  The cred_guard_mutex is
    held over "put_user(0, tsk->clear_child_tid)" in exit_mm().  The
    cred_guard_mutex is held over "get_user(futex_offset, ...")  in
    exit_robust_list.  The cred_guard_mutex held over copy_strings.
    
    The functions get_user and put_user can trigger a page fault which can
    potentially wait indefinitely in the case of userfaultfd or if
    userspace implements part of the page fault path.
    
    In any of those cases the userspace process that the kernel is waiting
    for might make a different system call that winds up taking the
    cred_guard_mutex and result in deadlock.
    
    Holding a mutex over any of those possibly indefinite waits for
    userspace does not appear necessary.  Add exec_update_mutex that will
    just cover updating the process during exec where the permissions and
    the objects pointed to by the task struct may be out of sync.
    
    The plan is to switch the users of cred_guard_mutex to
    exec_update_mutex one by one.  This lets us move forward while still
    being careful and not introducing any regressions.
    
    Link: https://lore.kernel.org/lkml/20160921152946.GA24210@dhcp22.suse.cz/
    Link: https://lore.kernel.org/lkml/AM6PR03MB5170B06F3A2B75EFB98D071AE4E60@AM6PR03MB5170.eurprd03.prod.outlook.com/
    Link: https://lore.kernel.org/linux-fsdevel/20161102181806.GB1112@redhat.com/
    Link: https://lore.kernel.org/lkml/20160923095031.GA14923@redhat.com/
    Link: https://lore.kernel.org/lkml/20170213141452.GA30203@redhat.com/
    Ref: 45c1a159b85b ("Add PTRACE_O_TRACEVFORKDONE and PTRACE_O_TRACEEXIT facilities.")
    Ref: 456f17cd1a28 ("[PATCH] user-vm-unlock-2.5.31-A2")
    Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>


Bernd do you want to give me your Reviewed-by for this part of the
series?

After that do you think you can write the obvious patch for mm_access?

I will apply these changes to my tree and push them into linux-next.

Eric