linux-kernel - Re: possible deadlock in send

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, 03 Apr 2020 07:57:04 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     Oleg Nesterov <oleg@...hat.com>,
        syzbot <syzbot+f675f964019f884dbd0f@...kaller.appspotmail.com>,
        adobriyan@...il.com, akpm@...ux-foundation.org,
        allison@...utok.net, areber@...hat.com, aubrey.li@...ux.intel.com,
        avagin@...il.com, bfields@...ldses.org, christian@...uner.io,
        cyphar@...har.com, gregkh@...uxfoundation.org, guro@...com,
        jlayton@...nel.org, joel@...lfernandes.org, keescook@...omium.org,
        linmiaohe@...wei.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, mhocko@...e.com, mingo@...nel.org,
        peterz@...radead.org, sargun@...gun.me,
        syzkaller-bugs@...glegroups.com, tglx@...utronix.de,
        viro@...iv.linux.org.uk
Subject: Re: possible deadlock in send_sigurg

Christian Brauner <christian.brauner@...ntu.com> writes:

> On Fri, Apr 03, 2020 at 11:11:35AM +0200, Oleg Nesterov wrote:
>> On 04/02, syzbot wrote:
>> >
>> >                       lock_acquire+0x1f2/0x8f0 kernel/locking/lockdep.c:4923
>> >                       __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
>> >                       _raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:151
>> >                       spin_lock include/linux/spinlock.h:353 [inline]
>> >                       proc_pid_make_inode+0x1f9/0x3c0 fs/proc/base.c:1880
>> 
>> Yes, spin_lock(wait_pidfd.lock) is not safe...
>> 
>> Eric, at first glance the fix is simple.
>> 
>> Oleg.
>> 
>> 
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>
> Um, when did this lock get added to proc/base.c in the first place and
> why has it been abused for this?

Because struct pid is too bloated already.

> People just recently complained loudly about this in the
> cred_guard_mutex thread that abusing locks for things they weren't
> intended for is a bad idea...

The problem there is/was holding locks over places they shouldn't.
It looks like I made an equally dump mistake with struct pid.

That said can you take a look at calling putting do_notify_pidfd
someplace sane.  I can't see how it makes sense to call that in
the same set of circumstances where we notify the parent.

Reparenting should not be a concern, nor should ptracing.  Which I think
means that do_notify_pid can potentially get called many times more
than it needs to be.

Not to mention it is being called a bit too soon when called from
do_notify_parent.  Which I saw earlier is causing problems.  Signal
sending can call do_notify_parent early because everything just queues
up and no action is taken.  Wake-ups on the other hand trigger more
immediate action.

There is no connection to the current bug except this discussion
just remimded me about do_notify_pidfd and I figured I should say
something before I forget again.

Eric