* [PATCH v2 1/2] pid: deinline kill_cad_pid @ 2026-07-18 3:19 Cen Zhang (Microsoft) 2026-07-18 3:20 ` [PATCH v2 2/2] pid: fix cad_pid use-after-free race Cen Zhang (Microsoft) 2026-07-18 10:24 ` [PATCH v2 1/2] pid: deinline kill_cad_pid Bradley Morgan 0 siblings, 2 replies; 10+ messages in thread From: Cen Zhang (Microsoft) @ 2026-07-18 3:19 UTC (permalink / raw) To: brauner Cc: oleg, akpm, jack, avagin, ptikhomirov, mjguzik, include, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, blbllhy Move kill_cad_pid() out of the header without changing behavior. This prepares for taking a reference to cad_pid under RCU in the following fix. Suggested-by: Mateusz Guzik <mjguzik@gmail.com> Suggested-by: Bradley Morgan <include@grrlz.net> Link: https://lore.kernel.org/all/CAGudoHH0vz=1m97EDHQFLLwGJmDPmqWJ+444b7rMiD059drEwQ@mail.gmail.com/ Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com> --- v2: - New preparatory patch to deinline kill_cad_pid(). include/linux/sched/signal.h | 5 +---- kernel/pid.c | 5 +++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index 584ae88b435e..d45a5476b97d 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -562,10 +562,7 @@ static inline sigset_t *sigmask_to_save(void) return res; } -static inline int kill_cad_pid(int sig, int priv) -{ - return kill_pid(cad_pid, sig, priv); -} +int kill_cad_pid(int sig, int priv); /* These can be the second arg to send_sig_info/send_group_sig_info. */ #define SEND_SIG_NOINFO ((struct kernel_siginfo *) 0) diff --git a/kernel/pid.c b/kernel/pid.c index f55189a3d07d..234ebee29375 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -557,6 +557,11 @@ pid_t pid_vnr(struct pid *pid) } EXPORT_SYMBOL_GPL(pid_vnr); +int kill_cad_pid(int sig, int priv) +{ + return kill_pid(cad_pid, sig, priv); +} + pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type, struct pid_namespace *ns) { -- 2.53.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 3:19 [PATCH v2 1/2] pid: deinline kill_cad_pid Cen Zhang (Microsoft) @ 2026-07-18 3:20 ` Cen Zhang (Microsoft) 2026-07-18 11:54 ` Bradley Morgan 2026-07-18 13:19 ` Oleg Nesterov 2026-07-18 10:24 ` [PATCH v2 1/2] pid: deinline kill_cad_pid Bradley Morgan 1 sibling, 2 replies; 10+ messages in thread From: Cen Zhang (Microsoft) @ 2026-07-18 3:20 UTC (permalink / raw) To: brauner Cc: oleg, akpm, jack, avagin, ptikhomirov, mjguzik, include, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, blbllhy, stable proc_do_cad_pid() reads the global cad_pid pointer and passes it to pid_vnr() without protecting the lifetime of the referenced struct pid. A concurrent writer can replace cad_pid and drop the final reference to the old struct pid after the reader has loaded the pointer but before pid_vnr() has finished dereferencing it, causing a use-after-free. The sysctl is mode 0600, but access is checked against the owning user namespace, so an unprivileged user can reach it via userns, pidns, and a proc mount. Fix this by treating cad_pid as an RCU-protected pointer at both read sites and by waiting for a grace period before dropping the old reference on the write side. KASAN crash stack: kernel/pid.c:545 pid_nr_ns() # reads freed pid->level kernel/pid.c:556 pid_vnr() kernel/pid.c:775 proc_do_cad_pid() fs/proc/proc_sysctl.c proc_sys_call_handler() fs/read_write.c vfs_read() fs/read_write.c __x64_sys_pread64() Fixes: 9ec52099e4b8 ("[PATCH] replace cad_pid by a struct pid") Reported-by: AutonomousCodeSecurity@microsoft.com Closes: https://lore.kernel.org/all/20260717210143.4734-1-blbllhy@gmail.com/ Suggested-by: Mateusz Guzik <mjguzik@gmail.com> Reviewed-by: Bradley Morgan <include@grrlz.net> Cc: stable@vger.kernel.org Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com> --- v2: - Split out kill_cad_pid() deinline into a preparatory patch. - Annotate cad_pid as __rcu and use rcu_dereference(). - Protect kill_cad_pid() by taking a pid reference under RCU. - Add a comment explaining why synchronize_rcu() is used instead of call_rcu(). include/linux/sched.h | 2 +- init/main.c | 2 +- kernel/pid.c | 26 +++++++++++++++++++++++--- kernel/reboot.c | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 373bcc0598d1..31ce72b1233c 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1767,7 +1767,7 @@ static inline bool is_lazy_mmu_mode_active(void) } #endif -extern struct pid *cad_pid; +extern struct pid __rcu *cad_pid; /* * Per process flags diff --git a/init/main.c b/init/main.c index e363232b428b..19a10d0c2760 100644 --- a/init/main.c +++ b/init/main.c @@ -1636,7 +1636,7 @@ static noinline void __init kernel_init_freeable(void) */ set_mems_allowed(node_states[N_MEMORY]); - cad_pid = get_pid(task_pid(current)); + rcu_assign_pointer(cad_pid, get_pid(task_pid(current))); smp_prepare_cpus(setup_max_cpus); diff --git a/kernel/pid.c b/kernel/pid.c index 234ebee29375..bffc5f765080 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -559,7 +559,17 @@ EXPORT_SYMBOL_GPL(pid_vnr); 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); + + return ret; } pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type, @@ -773,11 +783,15 @@ static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffe size_t *lenp, loff_t *ppos) { struct pid *new_pid; + struct pid *old_pid; pid_t tmp_pid; int r; struct ctl_table tmp_table = *table; - tmp_pid = pid_vnr(cad_pid); + rcu_read_lock(); + tmp_pid = pid_vnr(rcu_dereference(cad_pid)); + rcu_read_unlock(); + tmp_table.data = &tmp_pid; r = proc_dointvec(&tmp_table, write, buffer, lenp, ppos); @@ -788,7 +802,13 @@ static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffe if (!new_pid) return -ESRCH; - put_pid(xchg(&cad_pid, new_pid)); + old_pid = unrcu_pointer(xchg(&cad_pid, RCU_INITIALIZER(new_pid))); + /* + * Wait for cad_pid readers before put_pid(). We cannot use + * call_rcu() here because free_pid() already owns pid->rcu. + */ + synchronize_rcu(); + put_pid(old_pid); return 0; } diff --git a/kernel/reboot.c b/kernel/reboot.c index 695c33e75efd..fc191a48c0e9 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -24,7 +24,7 @@ */ static int C_A_D = 1; -struct pid *cad_pid; +struct pid __rcu *cad_pid; EXPORT_SYMBOL(cad_pid); #if defined(CONFIG_ARM) -- 2.53.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 3:20 ` [PATCH v2 2/2] pid: fix cad_pid use-after-free race Cen Zhang (Microsoft) @ 2026-07-18 11:54 ` Bradley Morgan 2026-07-18 13:19 ` Oleg Nesterov 1 sibling, 0 replies; 10+ messages in thread From: Bradley Morgan @ 2026-07-18 11:54 UTC (permalink / raw) To: Cen Zhang (Microsoft), brauner Cc: oleg, akpm, jack, avagin, ptikhomirov, mjguzik, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, blbllhy, stable On July 18, 2026 4:20:00 AM GMT+01:00, "Cen Zhang (Microsoft)" <blbllhy@gmail.com> wrote: >proc_do_cad_pid() reads the global cad_pid pointer and passes it to >pid_vnr() without protecting the lifetime of the referenced struct pid. >A concurrent writer can replace cad_pid and drop the final reference to >the old struct pid after the reader has loaded the pointer but before >pid_vnr() has finished dereferencing it, causing a use-after-free. > >The sysctl is mode 0600, but access is checked against the owning user >namespace, so an unprivileged user can reach it via userns, pidns, and a >proc mount. > >Fix this by treating cad_pid as an RCU-protected pointer at both read >sites and by waiting for a grace period before dropping the old reference >on the write side. > >KASAN crash stack: > kernel/pid.c:545 pid_nr_ns() # reads freed pid->level > kernel/pid.c:556 pid_vnr() > kernel/pid.c:775 proc_do_cad_pid() > fs/proc/proc_sysctl.c proc_sys_call_handler() > fs/read_write.c vfs_read() > fs/read_write.c __x64_sys_pread64() > >Fixes: 9ec52099e4b8 ("[PATCH] replace cad_pid by a struct pid") >Reported-by: AutonomousCodeSecurity@microsoft.com >Closes: https://lore.kernel.org/all/20260717210143.4734-1-blbllhy@gmail.com/ >Suggested-by: Mateusz Guzik <mjguzik@gmail.com> >Reviewed-by: Bradley Morgan <include@grrlz.net> This tag remains, no need to revoke. Reviewed-by: Bradley Morgan <include@grrlz.net> >Cc: stable@vger.kernel.org >Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com> >--- >v2: > - Split out kill_cad_pid() deinline into a preparatory patch. > - Annotate cad_pid as __rcu and use rcu_dereference(). > - Protect kill_cad_pid() by taking a pid reference under RCU. > - Add a comment explaining why synchronize_rcu() is used instead of > call_rcu(). > > include/linux/sched.h | 2 +- > init/main.c | 2 +- > kernel/pid.c | 26 +++++++++++++++++++++++--- > kernel/reboot.c | 2 +- > 4 files changed, 26 insertions(+), 6 deletions(-) > >diff --git a/include/linux/sched.h b/include/linux/sched.h >index 373bcc0598d1..31ce72b1233c 100644 >--- a/include/linux/sched.h >+++ b/include/linux/sched.h >@@ -1767,7 +1767,7 @@ static inline bool is_lazy_mmu_mode_active(void) > } > #endif > >-extern struct pid *cad_pid; >+extern struct pid __rcu *cad_pid; > > /* > * Per process flags >diff --git a/init/main.c b/init/main.c >index e363232b428b..19a10d0c2760 100644 >--- a/init/main.c >+++ b/init/main.c >@@ -1636,7 +1636,7 @@ static noinline void __init kernel_init_freeable(void) > */ > set_mems_allowed(node_states[N_MEMORY]); > >- cad_pid = get_pid(task_pid(current)); >+ rcu_assign_pointer(cad_pid, get_pid(task_pid(current))); > > smp_prepare_cpus(setup_max_cpus); > >diff --git a/kernel/pid.c b/kernel/pid.c >index 234ebee29375..bffc5f765080 100644 >--- a/kernel/pid.c >+++ b/kernel/pid.c >@@ -559,7 +559,17 @@ EXPORT_SYMBOL_GPL(pid_vnr); > > 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); >+ >+ return ret; > } > > pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type, >@@ -773,11 +783,15 @@ static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffe > size_t *lenp, loff_t *ppos) > { > struct pid *new_pid; >+ struct pid *old_pid; > pid_t tmp_pid; > int r; > struct ctl_table tmp_table = *table; > >- tmp_pid = pid_vnr(cad_pid); >+ rcu_read_lock(); >+ tmp_pid = pid_vnr(rcu_dereference(cad_pid)); >+ rcu_read_unlock(); >+ > tmp_table.data = &tmp_pid; > > r = proc_dointvec(&tmp_table, write, buffer, lenp, ppos); >@@ -788,7 +802,13 @@ static int proc_do_cad_pid(const struct ctl_table *table, int write, void *buffe > if (!new_pid) > return -ESRCH; > >- put_pid(xchg(&cad_pid, new_pid)); >+ old_pid = unrcu_pointer(xchg(&cad_pid, RCU_INITIALIZER(new_pid))); >+ /* >+ * Wait for cad_pid readers before put_pid(). We cannot use >+ * call_rcu() here because free_pid() already owns pid->rcu. >+ */ >+ synchronize_rcu(); >+ put_pid(old_pid); > return 0; > } > >diff --git a/kernel/reboot.c b/kernel/reboot.c >index 695c33e75efd..fc191a48c0e9 100644 >--- a/kernel/reboot.c >+++ b/kernel/reboot.c >@@ -24,7 +24,7 @@ > */ > > static int C_A_D = 1; >-struct pid *cad_pid; >+struct pid __rcu *cad_pid; > EXPORT_SYMBOL(cad_pid); > > #if defined(CONFIG_ARM) > Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 3:20 ` [PATCH v2 2/2] pid: fix cad_pid use-after-free race Cen Zhang (Microsoft) 2026-07-18 11:54 ` Bradley Morgan @ 2026-07-18 13:19 ` Oleg Nesterov 2026-07-18 13:59 ` Bradley Morgan 2026-07-18 15:37 ` Mateusz Guzik 1 sibling, 2 replies; 10+ messages in thread From: Oleg Nesterov @ 2026-07-18 13:19 UTC (permalink / raw) To: Cen Zhang (Microsoft) Cc: brauner, akpm, jack, avagin, ptikhomirov, mjguzik, include, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable 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? Oleg. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 13:19 ` Oleg Nesterov @ 2026-07-18 13:59 ` Bradley Morgan 2026-07-18 14:13 ` Oleg Nesterov 2026-07-18 15:37 ` Mateusz Guzik 1 sibling, 1 reply; 10+ messages in thread From: Bradley Morgan @ 2026-07-18 13:59 UTC (permalink / raw) To: Oleg Nesterov, Cen Zhang (Microsoft) Cc: brauner, akpm, jack, avagin, ptikhomirov, mjguzik, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable On July 18, 2026 2:19:57 PM GMT+01:00, 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? > >Oleg. > > On 07/18, Oleg Nesterov wrote: > 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? youre right that the get/put is redundant. kill_pid() doesnt sleep, it just grabs tasklist_lock, so doing it inside the read section is fine and the extra ref buys nothing. but i dont think either form is actually safe, and thats the bit id like Cen to address. rcu_read_lock() only protects the pid if the freeing side defers to a grace period. on the cad_pid path it doesnt: proc_do_cad_pid(): put_pid(xchg(&cad_pid, new_pid)); put_pid(): if (refcount_dec_and_test(&pid->count)) { pidfs_free_pid(pid); kmem_cache_free(ns->pid_cachep, pid); /* synchronous */ put_pid_ns(ns); } thats a straight kmem_cache_free, no call_rcu. only free_pid() goes through call_rcu(delayed_put_pid), and thats dropping the hash reference, not the reference cad_pid holds. and the pid cache isnt SLAB_TYPESAFE_BY_RCU: kmem_cache_create("pid", ..., SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT, NULL); so: cpu0 kill_cad_pid cpu1 proc_do_cad_pid rcu_read_lock() p = rcu_dereference(cad_pid) put_pid(old) // last ref, freed and reused kill_pid(p, ...) // UAF rcu_read_unlock() your form and Cens both hit this. Cens get_pid() is no safer either, its a refcount_inc on the same p thats already freed, so its a UAF too, not a more careful variant. to make the rcu_read_lock() mean anything the writer has to defer. either proc_do_cad_pid() drops the old pid via call_rcu instead of a bare put_pid(), or the pid cache goes SLAB_TYPESAFE_BY_RCU and the reader uses a not zero tryget. with the writer fixed your clean form is exactly right and the ref stays unnecessary. its reachable when cad_pid isnt init: root writes a pid, that task exits, cad_pid ends up holding the last ref, and the next write frees it while a signal is in flight. Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 13:59 ` Bradley Morgan @ 2026-07-18 14:13 ` Oleg Nesterov 2026-07-18 14:28 ` Bradley Morgan 0 siblings, 1 reply; 10+ messages in thread From: Oleg Nesterov @ 2026-07-18 14:13 UTC (permalink / raw) To: Bradley Morgan Cc: Cen Zhang (Microsoft), brauner, akpm, jack, avagin, ptikhomirov, mjguzik, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable On 07/18, Bradley Morgan wrote: > > On July 18, 2026 2:19:57 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com> > > > 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? > > youre right that the get/put is redundant. kill_pid() doesnt sleep, it just > grabs tasklist_lock, (no it doesn't take tasklist) > but i dont think either form is actually safe, and thats the bit id like > Cen to address. rcu_read_lock() only protects the pid if the freeing side > defers to a grace period. on the cad_pid path it doesnt: > > proc_do_cad_pid(): > put_pid(xchg(&cad_pid, new_pid)); I must have missed something. But this patch adds synchronize_rcu() before the final put_pid() ? And. If kill_pid(rcu_dereference(cad_pid)) was not safe under rcu_read_lock(), then get_pid(rcu_dereference(cad_pid)) would be equally unsafe? Oleg. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 14:13 ` Oleg Nesterov @ 2026-07-18 14:28 ` Bradley Morgan 0 siblings, 0 replies; 10+ messages in thread From: Bradley Morgan @ 2026-07-18 14:28 UTC (permalink / raw) To: Oleg Nesterov Cc: Cen Zhang (Microsoft), brauner, akpm, jack, avagin, ptikhomirov, mjguzik, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable On July 18, 2026 3:13:37 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com> wrote: >On 07/18, Bradley Morgan wrote: >> >> On July 18, 2026 2:19:57 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com> >> >> > 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? >> >> youre right that the get/put is redundant. kill_pid() doesnt sleep, it >just >> grabs tasklist_lock, > >(no it doesn't take tasklist) > >> but i dont think either form is actually safe, and thats the bit id like >> Cen to address. rcu_read_lock() only protects the pid if the freeing >side >> defers to a grace period. on the cad_pid path it doesnt: >> >> proc_do_cad_pid(): >> put_pid(xchg(&cad_pid, new_pid)); > >I must have missed something. > >But this patch adds synchronize_rcu() before the final put_pid() ? > >And. If kill_pid(rcu_dereference(cad_pid)) was not safe under >rcu_read_lock(), >then get_pid(rcu_dereference(cad_pid)) would be equally unsafe? > >Oleg. > > oops, my bad. Brainfog. Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 13:19 ` Oleg Nesterov 2026-07-18 13:59 ` Bradley Morgan @ 2026-07-18 15:37 ` Mateusz Guzik 2026-07-18 15:58 ` Oleg Nesterov 1 sibling, 1 reply; 10+ messages in thread From: Mateusz Guzik @ 2026-07-18 15:37 UTC (permalink / raw) To: Oleg Nesterov Cc: Cen Zhang (Microsoft), brauner, akpm, jack, avagin, ptikhomirov, include, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable 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. The separate patch which deinlines kill_cad_pid should be folded into the actual fix, no point having 2 patches for this one. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] pid: fix cad_pid use-after-free race 2026-07-18 15:37 ` Mateusz Guzik @ 2026-07-18 15:58 ` Oleg Nesterov 0 siblings, 0 replies; 10+ messages in thread From: Oleg Nesterov @ 2026-07-18 15:58 UTC (permalink / raw) To: Mateusz Guzik Cc: Cen Zhang (Microsoft), brauner, akpm, jack, avagin, ptikhomirov, include, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, stable 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] pid: deinline kill_cad_pid 2026-07-18 3:19 [PATCH v2 1/2] pid: deinline kill_cad_pid Cen Zhang (Microsoft) 2026-07-18 3:20 ` [PATCH v2 2/2] pid: fix cad_pid use-after-free race Cen Zhang (Microsoft) @ 2026-07-18 10:24 ` Bradley Morgan 1 sibling, 0 replies; 10+ messages in thread From: Bradley Morgan @ 2026-07-18 10:24 UTC (permalink / raw) To: Cen Zhang (Microsoft), brauner Cc: oleg, akpm, jack, avagin, ptikhomirov, mjguzik, linux-kernel, AutonomousCodeSecurity, tgopinath, kys, blbllhy On July 18, 2026 4:19:59 AM GMT+01:00, "Cen Zhang (Microsoft)" <blbllhy@gmail.com> wrote: >Move kill_cad_pid() out of the header without changing behavior. This >prepares for taking a reference to cad_pid under RCU in the following >fix. > >Suggested-by: Mateusz Guzik <mjguzik@gmail.com> >Suggested-by: Bradley Morgan <include@grrlz.net> >Link: https://lore.kernel.org/all/CAGudoHH0vz=1m97EDHQFLLwGJmDPmqWJ+444b7rMiD059drEwQ@mail.gmail.com/ >Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com> LGTM! Please add Reviewed-by: Bradley Morgan <include@grrlz.net> >--- >v2: > - New preparatory patch to deinline kill_cad_pid(). > > include/linux/sched/signal.h | 5 +---- > kernel/pid.c | 5 +++++ > 2 files changed, 6 insertions(+), 4 deletions(-) > >diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h >index 584ae88b435e..d45a5476b97d 100644 >--- a/include/linux/sched/signal.h >+++ b/include/linux/sched/signal.h >@@ -562,10 +562,7 @@ static inline sigset_t *sigmask_to_save(void) > return res; > } > >-static inline int kill_cad_pid(int sig, int priv) >-{ >- return kill_pid(cad_pid, sig, priv); >-} >+int kill_cad_pid(int sig, int priv); > > /* These can be the second arg to send_sig_info/send_group_sig_info. */ > #define SEND_SIG_NOINFO ((struct kernel_siginfo *) 0) >diff --git a/kernel/pid.c b/kernel/pid.c >index f55189a3d07d..234ebee29375 100644 >--- a/kernel/pid.c >+++ b/kernel/pid.c >@@ -557,6 +557,11 @@ pid_t pid_vnr(struct pid *pid) > } > EXPORT_SYMBOL_GPL(pid_vnr); > >+int kill_cad_pid(int sig, int priv) >+{ >+ return kill_pid(cad_pid, sig, priv); >+} >+ > pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type, > struct pid_namespace *ns) > { > Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-18 15:58 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-18 3:19 [PATCH v2 1/2] pid: deinline kill_cad_pid Cen Zhang (Microsoft) 2026-07-18 3:20 ` [PATCH v2 2/2] pid: fix cad_pid use-after-free race Cen Zhang (Microsoft) 2026-07-18 11:54 ` Bradley Morgan 2026-07-18 13:19 ` Oleg Nesterov 2026-07-18 13:59 ` Bradley Morgan 2026-07-18 14:13 ` Oleg Nesterov 2026-07-18 14:28 ` Bradley Morgan 2026-07-18 15:37 ` Mateusz Guzik 2026-07-18 15:58 ` Oleg Nesterov 2026-07-18 10:24 ` [PATCH v2 1/2] pid: deinline kill_cad_pid Bradley Morgan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.