All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Gladkov <legion@kernel.org>
To: Sasha Levin <sashal@kernel.org>
Cc: nickolai@csail.mit.edu, brauner@kernel.org, stable@vger.kernel.org
Subject: Re: FAILED: patch "[PATCH] ucount: take the rlimit reference before checking the limit" failed to apply to 5.15-stable tree
Date: Sat, 7 Nov 2026 14:56:46 +0100	[thread overview]
Message-ID: <au8uHh_iq7Ojywef@example.org> (raw)
In-Reply-To: <20261107061036-failed-5.15-b14fcabd34e0@kernel.org>

On Sat, Nov 07, 2026 at 06:10:36AM -0500, Sasha Levin wrote:
> The patch below does not apply to the 5.15-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> To reproduce the conflict and resubmit, you may use the following commands:
> 
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
> git checkout FETCH_HEAD
> git cherry-pick -x b14fcabd34e02c40f72d3cd1e9bd4fd18df40ff1
> # <resolve conflicts, build, test, etc.>
> git commit -s
> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '20261107061036-failed-5.15-b14fcabd34e0@kernel.org' --subject-prefix 'PATCH 5.15.y' 'HEAD^..'

The 5.15-stable is too old for this change. The branch does not contain
commits that separate ucounts and rlimits.

9e05e5c7ee87 ("signal: restore the override_rlimit logic")
de399236e240 ("ucounts: Split rlimit and ucount values and max values")
(and most likely there are still a few more.)

I don't think it's worth backporting these changes.

> Thanks,
> Sasha
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From b14fcabd34e02c40f72d3cd1e9bd4fd18df40ff1 Mon Sep 17 00:00:00 2001
> From: Nickolai Zeldovich <nickolai@csail.mit.edu>
> Date: Mon, 19 Oct 2026 14:59:13 -0400
> Subject: [PATCH] ucount: take the rlimit reference before checking the limit
> 
> inc_rlimit_get_ucounts() takes the reference that pins a ucounts level
> while its rlimit count is non-zero only in the walk whose increment is
> the 0 -> 1 transition, and only after that walk passed the limit check.
> A walk that raises the count from zero and then fails the limit backs
> out without the reference, while a concurrent walk that raised it past
> one keeps its increment and never takes one either:
> 
>   CPU0 (obeys the limit)              CPU1 (overrides it)
>   new = add_return(1) -> 1
>                                       new = add_return(1) -> 2
>                                       new != 1, no get_ucounts()
>   new > max -> dec_unwind
>   sub_return(1) -> 1   (no put)
> 
> The count is 1 with no reference held. When CPU1 drops its charge,
> dec_rlimit_put_ucounts() takes the count to 0 and puts a reference
> nobody took, which frees the live struct ucounts, and the user_namespace
> it pins, with kfree_rcu(). The mirror interleaving leaks the reference
> instead.
> 
> An unprivileged user reaches this on a kernel with CONFIG_USER_NS=y: in
> a user namespace with RLIMIT_SIGPENDING 0, one process taking
> synchronous SIGSEGVs (forced, so override_rlimit is set) races a second
> process in the same namespace and uid that queues real-time signals,
> which obey the limit. The two hold different sighand->siglock but charge
> the same per-namespace SIGPENDING ucounts. Unpatched, with rcuref
> debugging:
> 
>   rcuref - imbalanced put()
>   WARNING: lib/rcuref.c:266 at rcuref_put_slowpath+0xf4/0x120
>    put_ucounts+0x5a/0x180
>    do_dec_rlimit_put_ucounts+0x70/0x90
>    __sigqueue_free+0x33/0xc0
>    get_signal+0x56f/0xd20
>    arch_do_signal_or_restart+0x87/0x380
>    irqentry_exit+0x243/0x650
>    asm_exc_page_fault+0x26/0x30
> 
> Take the reference on the 0 -> 1 transition before the limit is checked,
> so the walk that raises the count from zero owns that level's reference
> even when its request is rejected, and unwind a rejected level through
> do_dec_rlimit_put_ucounts(), which releases the reference when the count
> returns to 0. The get_ucounts() failure path is unchanged. With the
> patch the same interleaving, forced some 80000 times in a KASAN build,
> produces no warning and no KASAN report.
> 
> Fixes: 15bc01effefe ("ucounts: Fix signal ucount refcounting")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
> Link: https://patch.msgid.link/20261019185913.805378-1-nickolai@csail.mit.edu
> Acked-by: Alexey Gladkov <legion@kernel.org>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  kernel/ucount.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index ec8b1445e2877..c3de5b94d25f7 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -308,23 +308,26 @@ long inc_rlimit_get_ucounts(struct ucounts *ucounts, enum rlimit_type type,
>  
>  	for (iter = ucounts; iter; iter = iter->ns->ucounts) {
>  		long new = atomic_long_add_return(1, &iter->rlimit[type]);
> -		if (new < 0 || new > max)
> +		/*
> +		 * Take the extra reference on the 0 -> 1 transition before the
> +		 * limit check: a rejected walk must not leave a concurrent one
> +		 * holding the surviving increment with no reference.
> +		 */
> +		if (new == 1 && !get_ucounts(iter))
>  			goto dec_unwind;
> +		if (new < 0 || new > max) {
> +			/* undo iter too, releasing the reference taken above */
> +			do_dec_rlimit_put_ucounts(ucounts, iter->ns->ucounts, type);
> +			return 0;
> +		}
>  		if (iter == ucounts)
>  			ret = new;
>  		if (!override_rlimit)
>  			max = get_userns_rlimit_max(iter->ns, type);
> -		/*
> -		 * Grab an extra ucount reference for the caller when
> -		 * the rlimit count was previously 0.
> -		 */
> -		if (new != 1)
> -			continue;
> -		if (!get_ucounts(iter))
> -			goto dec_unwind;
>  	}
>  	return ret;
>  dec_unwind:
> +	/* get_ucounts() failed for iter: undo iter without a put */
>  	dec = atomic_long_sub_return(1, &iter->rlimit[type]);
>  	WARN_ON_ONCE(dec < 0);
>  	do_dec_rlimit_put_ucounts(ucounts, iter, type);
> -- 
> 2.53.0
> 
> 
> 
> 

-- 
Rgrds, legion


  reply	other threads:[~2026-11-07 13:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-07 11:10 FAILED: patch "[PATCH] ucount: take the rlimit reference before checking the limit" failed to apply to 5.15-stable tree Sasha Levin
2026-11-07 13:56 ` Alexey Gladkov [this message]
2026-11-10 22:41 ` [PATCH 5.15.y 1/4] ucounts: Use atomic_long_sub_return for clarity Sasha Levin
2026-11-10 22:41   ` [PATCH 5.15.y 2/4] ucounts: Split rlimit and ucount values and max values Sasha Levin
2026-11-10 22:41   ` [PATCH 5.15.y 3/4] signal: restore the override_rlimit logic Sasha Levin
2026-11-10 22:41   ` [PATCH 5.15.y 4/4] ucount: take the rlimit reference before checking the limit Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=au8uHh_iq7Ojywef@example.org \
    --to=legion@kernel.org \
    --cc=brauner@kernel.org \
    --cc=nickolai@csail.mit.edu \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.