[PATCH v1] ucount: read the parent ucounts before dropping the rlimit count

[PATCH v1] ucount: read the parent ucounts before dropping the rlimit count

[Nickolai Zeldovich]

do_dec_rlimit_put_ucounts() drops its share of iter->rlimit[type] and
only then loads iter->ns->ucounts. If the decrement leaves the count
non-zero, a concurrent caller can take it to zero and put the last
reference, which frees the ucounts and possibly its user namespace with
kfree_rcu(). An exiting task frees its pending signals without siglock
or any RCU read-side section, so when it is preempted between the
decrement and the load for a grace period it reads freed memory and
then decrements a counter through the pointer it found there.

A pending signal holds no reference on its ucounts: the reference is
taken by whoever moves the count from 0 to 1 and dropped by whoever
moves it back to 0, so the caller's share of the count is all that
keeps iter alive, and only until it is dropped.

Load the parent first; it stays pinned by the caller's share of its
own count.

Fixes: 15bc01effefe ("ucounts: Fix signal ucount refcounting")
Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---

Notes:
    Callers that run the loop outside any RCU read-side section on master:
    exit_signals() (flush_sigqueue_list() after dropping siglock),
    release_task() (flush_sigqueue() of shared_pending) and
    posixtimer_free_timer() (timer_delete(), posixtimer_delete_timers()).
    The callers that hold siglock run with interrupts off, and since
    put_ucounts() and free_user_ns() free with kfree_rcu() the objects
    outlive them. Before commit 5f01a22c5b23 ("ucount: use RCU for ucounts
    lookups") put_ucounts() used kfree(), so the window was open for every
    caller.
    
    The interleaving, on a ucounts whose SIGPENDING count is 2 and which no
    cred references any more:
    
      CPU0 (exiting task)              CPU1
      dec = sub_return(1) -> 1
      <preempted>
                                       dec = sub_return(1) -> 0
                                       put_ucounts(iter): last reference,
                                         kfree_rcu(iter)
                                       <grace period ends>
      next = iter->ns->ucounts  (freed)
    
    Loading first is enough: until the decrement, the caller's share
    keeps the count at 1 or more, so the reference taken on the 0 -> 1
    transition is still held; iter->ns is pinned by iter, ns->ucounts
    by ns, and next by the caller's undecremented share at its level.
    atomic_long_sub_return() is fully ordered, so the loads complete
    before the decrement is visible. rcu_read_lock() around the loop
    would also do on master, but only because both objects are freed
    with kfree_rcu().
    
    Reproduced on x86_64 under QEMU with the program below on a KASAN kernel
    (CONFIG_KASAN=y, CONFIG_USER_NS=y) with a test-only delay injected in the
    window between the decrement and the load in do_dec_rlimit_put_ucounts()
    and a kvfree_rcu_barrier() added after put_ucounts() so the freed node is
    reclaimed inside the window; the unpatched kernel reports
    
      BUG: KASAN: slab-use-after-free in do_dec_rlimit_put_ucounts
      Read of size 8 ... by task ucdprace
       do_dec_rlimit_put_ucounts
       __sigqueue_free
       exit_signals
       do_exit
      Allocated by task ...: alloc_ucounts <- set_cred_ucounts <- __sys_setresuid
      Freed by task 0: __kasan_slab_free <- rcu ... (kfree_rcu callback)
    
    and the patched kernel, with the same delay moved to just after the load,
    reports nothing.
    
    Reproducer (needs CAP_SETUID; an unprivileged user reaches it in a user
    namespace that maps more than one uid; cc -O2 -static -o race race.c):
    
      #define _GNU_SOURCE
      #include <sys/prctl.h>
      #include <sys/syscall.h>
      #include <sys/wait.h>
      #include <signal.h>
      #include <unistd.h>
      #define U 4000
      static void worker(int rfd)
      {
      	sigset_t set;
      	siginfo_t info = { .si_signo = SIGRTMIN, .si_code = SI_QUEUE };
      	char go;
      	prctl(PR_SET_NAME, "ucdprace", 0, 0, 0);
      	sigemptyset(&set);
      	sigaddset(&set, SIGRTMIN);
      	sigprocmask(SIG_BLOCK, &set, 0);
      	setresuid(U, U, 0);
      	syscall(SYS_rt_tgsigqueueinfo, getpid(), syscall(SYS_gettid),
      		SIGRTMIN, &info);
      	setresuid(0, 0, 0);
      	read(rfd, &go, 1);
      	_exit(0);
      }
      static pid_t spawn(int *go)
      {
      	int p[2];
      	pid_t pid;
      	pipe(p);
      	if ((pid = fork()) == 0) { close(p[1]); worker(p[0]); }
      	close(p[0]);
      	*go = p[1];
      	return pid;
      }
      int main(void)
      {
      	int ga, gb;
      	pid_t a = spawn(&ga), b = spawn(&gb);
      	usleep(200000);
      	write(ga, "x", 1);
      	usleep(100000);
      	write(gb, "x", 1);
      	waitpid(a, 0, 0);
      	waitpid(b, 0, 0);
      	return 0;
      }
    
    This patch was written with the assistance of an LLM.
    
 kernel/ucount.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/ucount.c b/kernel/ucount.c
index ec8b1445e2877..c91840cab8b85 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -285,9 +285,12 @@ static void do_dec_rlimit_put_ucounts(struct ucounts *ucounts,
 {
 	struct ucounts *iter, *next;
 	for (iter = ucounts; iter != last; iter = next) {
-		long dec = atomic_long_sub_return(1, &iter->rlimit[type]);
-		WARN_ON_ONCE(dec < 0);
+		long dec;
+
+		/* Our count pins @iter; once it is dropped @iter may be freed. */
 		next = iter->ns->ucounts;
+		dec = atomic_long_sub_return(1, &iter->rlimit[type]);
+		WARN_ON_ONCE(dec < 0);
 		if (dec == 0)
 			put_ucounts(iter);
 	}

base-commit: 26344c0dd32af8790d8474d233619db825441138
-- 
2.53.0

Re: [PATCH v1] ucount: read the parent ucounts before dropping the rlimit count

[Bradley Morgan]

On 24 October 2026 02:55:13 BST, Nickolai Zeldovich
<nickolai@csail.mit.edu> wrote:
>do_dec_rlimit_put_ucounts() drops its share of iter->rlimit[type] and
>only then loads iter->ns->ucounts. If the decrement leaves the count
>non-zero, a concurrent caller can take it to zero and put the last
>reference, which frees the ucounts and possibly its user namespace with
>kfree_rcu(). An exiting task frees its pending signals without siglock
>or any RCU read-side section, so when it is preempted between the
>decrement and the load for a grace period it reads freed memory and
>then decrements a counter through the pointer it found there.
>
>A pending signal holds no reference on its ucounts: the reference is
>taken by whoever moves the count from 0 to 1 and dropped by whoever
>moves it back to 0, so the caller's share of the count is all that
>keeps iter alive, and only until it is dropped.
>
>Load the parent first; it stays pinned by the caller's share of its
>own count.
>
>Fixes: 15bc01effefe ("ucounts: Fix signal ucount refcounting")

Ok.

>Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
>---
>
>Notes:
>    Callers that run the loop outside any RCU read-side section on master:
>    exit_signals() (flush_sigqueue_list() after dropping siglock),
>    release_task() (flush_sigqueue() of shared_pending) and
>    posixtimer_free_timer() (timer_delete(), posixtimer_delete_timers()).
>    The callers that hold siglock run with interrupts off, and since
>    put_ucounts() and free_user_ns() free with kfree_rcu() the objects
>    outlive them. Before commit 5f01a22c5b23 ("ucount: use RCU for ucounts
>    lookups") put_ucounts() used kfree(), so the window was open for every
>    caller.
>    
>    The interleaving, on a ucounts whose SIGPENDING count is 2 and which no
>    cred references any more:
>    
>      CPU0 (exiting task)              CPU1
>      dec = sub_return(1) -> 1
>      <preempted>
>                                       dec = sub_return(1) -> 0
>                                       put_ucounts(iter): last reference,
>                                         kfree_rcu(iter)
>                                       <grace period ends>
>      next = iter->ns->ucounts  (freed)
>    
>    Loading first is enough: until the decrement, the caller's share
>    keeps the count at 1 or more, so the reference taken on the 0 -> 1
>    transition is still held; iter->ns is pinned by iter, ns->ucounts
>    by ns, and next by the caller's undecremented share at its level.
>    atomic_long_sub_return() is fully ordered, so the loads complete
>    before the decrement is visible. rcu_read_lock() around the loop
>    would also do on master, but only because both objects are freed
>    with kfree_rcu().
>    
>    Reproduced on x86_64 under QEMU with the program below on a KASAN kernel
>    (CONFIG_KASAN=y, CONFIG_USER_NS=y) with a test-only delay injected in the
>    window between the decrement and the load in do_dec_rlimit_put_ucounts()
>    and a kvfree_rcu_barrier() added after put_ucounts() so the freed node is
>    reclaimed inside the window; the unpatched kernel reports
>    
>      BUG: KASAN: slab-use-after-free in do_dec_rlimit_put_ucounts
>      Read of size 8 ... by task ucdprace
>       do_dec_rlimit_put_ucounts
>       __sigqueue_free
>       exit_signals
>       do_exit
>      Allocated by task ...: alloc_ucounts <- set_cred_ucounts <- __sys_setresuid
>      Freed by task 0: __kasan_slab_free <- rcu ... (kfree_rcu callback)
>    
>    and the patched kernel, with the same delay moved to just after the load,
>    reports nothing.
>    
>    Reproducer (needs CAP_SETUID; an unprivileged user reaches it in a user
>    namespace that maps more than one uid; cc -O2 -static -o race race.c):
>    
>      #define _GNU_SOURCE
>      #include <sys/prctl.h>
>      #include <sys/syscall.h>
>      #include <sys/wait.h>
>      #include <signal.h>
>      #include <unistd.h>
>      #define U 4000
>      static void worker(int rfd)
>      {
>      	sigset_t set;
>      	siginfo_t info = { .si_signo = SIGRTMIN, .si_code = SI_QUEUE };
>      	char go;
>      	prctl(PR_SET_NAME, "ucdprace", 0, 0, 0);
>      	sigemptyset(&set);
>      	sigaddset(&set, SIGRTMIN);
>      	sigprocmask(SIG_BLOCK, &set, 0);
>      	setresuid(U, U, 0);
>      	syscall(SYS_rt_tgsigqueueinfo, getpid(), syscall(SYS_gettid),
>      		SIGRTMIN, &info);
>      	setresuid(0, 0, 0);
>      	read(rfd, &go, 1);
>      	_exit(0);
>      }
>      static pid_t spawn(int *go)
>      {
>      	int p[2];
>      	pid_t pid;
>      	pipe(p);
>      	if ((pid = fork()) == 0) { close(p[1]); worker(p[0]); }
>      	close(p[0]);
>      	*go = p[1];
>      	return pid;
>      }
>      int main(void)
>      {
>      	int ga, gb;
>      	pid_t a = spawn(&ga), b = spawn(&gb);
>      	usleep(200000);
>      	write(ga, "x", 1);
>      	usleep(100000);
>      	write(gb, "x", 1);
>      	waitpid(a, 0, 0);
>      	waitpid(b, 0, 0);
>      	return 0;
>      }
>    
>    This patch was written with the assistance of an LLM.
>    
> kernel/ucount.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/kernel/ucount.c b/kernel/ucount.c
>index ec8b1445e2877..c91840cab8b85 100644
>--- a/kernel/ucount.c
>+++ b/kernel/ucount.c
>@@ -285,9 +285,12 @@ static void do_dec_rlimit_put_ucounts(struct ucounts *ucounts,
> {
> 	struct ucounts *iter, *next;
> 	for (iter = ucounts; iter != last; iter = next) {
>-		long dec = atomic_long_sub_return(1, &iter->rlimit[type]);
>-		WARN_ON_ONCE(dec < 0);
>+		long dec;
>+
>+		/* Our count pins @iter; once it is dropped @iter may be freed. */
> 		next = iter->ns->ucounts;
>+		dec = atomic_long_sub_return(1, &iter->rlimit[type]);
>+		WARN_ON_ONCE(dec < 0);

Again, makes sense, cheers for the patch.

Reviewed-by: Bradley Morgan <brads@mainlining.org>


> 		if (dec == 0)
> 			put_ucounts(iter);
> 	}
>
>base-commit: 26344c0dd32af8790d8474d233619db825441138
>

--- Thanks!

Re: [PATCH v1] ucount: read the parent ucounts before dropping the rlimit count

[Christian Brauner]

On Fri, 23 Oct 2026 21:55:13 -0400, Nickolai Zeldovich wrote:
> do_dec_rlimit_put_ucounts() drops its share of iter->rlimit[type] and
> only then loads iter->ns->ucounts. If the decrement leaves the count
> non-zero, a concurrent caller can take it to zero and put the last
> reference, which frees the ucounts and possibly its user namespace with
> kfree_rcu(). An exiting task frees its pending signals without siglock
> or any RCU read-side section, so when it is preempted between the
> decrement and the load for a grace period it reads freed memory and
> then decrements a counter through the pointer it found there.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] ucount: read the parent ucounts before dropping the rlimit count
      https://git.kernel.org/vfs/vfs/c/38bf603d1ea5