Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race

[Oleg Nesterov <oleg@redhat.com>]

On 07/18, Mateusz Guzik wrote:
>
> On Sat, Jul 18, 2026 at 3:20 PM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 07/17, Cen Zhang (Microsoft) wrote:
> > >
> > >  int kill_cad_pid(int sig, int priv)
> > >  {
> > > -     return kill_pid(cad_pid, sig, priv);
> > > +     struct pid *pid;
> > > +     int ret;
> > > +
> > > +     rcu_read_lock();
> > > +     pid = get_pid(rcu_dereference(cad_pid));
> > > +     rcu_read_unlock();
> > > +
> > > +     ret = kill_pid(pid, sig, priv);
> > > +     put_pid(pid);
> >
> > Hmm. Why do we need get/put_pid() ? I think that
> >
> >         rcu_read_lock();
> >         ret = kill_pid(rcu_dereference(cad_pid), sig, priv);
> >         rcu_read_unlock();
> >
> >         return ret;
> >
> > should work just fine?
> >
>
> I suggested taking the ref as a future-proofing measure to *not*
> impose any requirements on kill_pid.
>
> Admittedly when responding to v1 I blindly assumed pid is always
> rcu-freed, my bad.
>
> Given the arcane nature of the proc file at hand and 0 performance
> concerns, I think the easiest way out is to give it a spinlock to
> protect access to cad_pid as opposed to trying to play any rcu and
> lockless games.

Just in case, I am fine either way. I'd personaly prefer to play rcu
games in this case, but I won't insist.

I've sent my comment only because get_pid + put_pid + kill_pid() outside
of rcu_read_lock() look really confusing to me.

As for requirements on kill_pid... See for example kill_pid_info().
kill_pid/kill_pid_info/etc are all rcu safe wrt "struct pid *pid" arg.

Oleg.