Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race
From: Oleg Nesterov
Date: Sat Jul 18 2026 - 11:59:16 EST
On 07/18, Mateusz Guzik wrote:
>
> On Sat, Jul 18, 2026 at 3:20 PM Oleg Nesterov <oleg@xxxxxxxxxx> 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.