linux-kernel - Re: [PATCH] kernel: Prevent prctl(PR_SET_PDEATHSIG) from racing with parent process exit

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: <2a3b72a8-6604-4c7e-a9b1-0c770efb29c6@gmail.com>
Date: Fri, 26 Sep 2025 19:58:05 -0400
From: Demi Marie Obenour <demiobenour@...il.com>
To: Mateusz Guzik <mjguzik@...il.com>, Oleg Nesterov <oleg@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Christian Brauner <brauner@...nel.org>,
 Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel: Prevent prctl(PR_SET_PDEATHSIG) from racing with
 parent process exit

On 9/25/25 14:35, Mateusz Guzik wrote:
> It struck me that this mail thread is perhaps a little rude towards
> Demi, so I would like to state the reported race is legitimate and if
> it was reported against come core functionality it would count as
> "good spotting". It just so happens this is a corner case to something
> not-that-imporant and the proposed fix is rather heavy-weight (despite
> being perfectly sensible), so there is quite a bit of reluctance.

That makes sense.  Thank you!

My personal thought is that prctl(PR_SET_DEATHSIG) is rather rare,
and also the lock is not held very long.  In particular, exit
already takes tasklist_lock for writing, and that is much more
common.  Therefore, I would be shocked if this added any significant
contention outside of contrived benchmarks.

What I _am_ concerned about is potential starvation,
especially on PREEMPT_RT.  Per the documentation:

> - Because an rwlock_t writer cannot grant its priority to multiple
>   readers, a preempted low-priority reader will continue holding its lock,
>   thus starving even high-priority writers.

This allows any user to hammer tasklist_lock
at will.  Is that going to be a problem?

> With that out of the way...
> 
> On Thu, Sep 25, 2025 at 6:29 PM Oleg Nesterov <oleg@...hat.com> wrote:
>>> That's very
>>> nasty as the full fence is quite expensive.
>>
>> Well, the exit_notify() path is already heavy, not sure smp_mb() or
>> smp_store_mb(real_parent, reaper) can add a noticeable difference.
>>
> 
> Well the tasklist consumers already suffer a lot of avoidable
> overhead, but I'm going to save the spiel about it. Maybe instead I
> will post a patch to remove some. ;)
> 
> I realized I never checked how often processes are exiting while still
> having children -- for legitimate workloads this is probably not that
> common either, so the fence would not even show up in typical usage?
> 
> This could be answered with bpftrace over a bunch of workloads if
> someone cares to investigate.
Starvation on PREEMPT_RT is my only concern.  See above.

>>> I don't know if makes any sense to add this.
>>
>> Neither me.
>>
>> OK. I won't argue with this patch. At least the usage of tasklist_lock is well
>> documented.
>>
> 
> ye.. avoiding smp_mb may be a case of "premature optimization", except
> it is also simpler, so that's a really tough call. good news is that
> it's not mine to make ;-)
> 
> I guess if the lock acquire goes in the sky is not going to fall,
> worst case this can get revisited later. So fwiw I would be leaning
> towards accepting the patch as well for the time being.

Thank you!
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

Download attachment "OpenPGP_0xB288B55FFF9C22C1.asc" of type "application/pgp-keys" (7141 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (834 bytes)