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

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2733 bytes --]

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@redhat.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)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]