Linus,
Please pull the ucount-rlimit-fixes-for-v5.17-rc2 branch from the git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git ucount-rlimit-fixes-for-v5.17-rc2
HEAD: f9d87929d451d3e649699d0f1d74f71f77ad38f5 ucount: Make get_ucount a safe get_user replacement
From: "Eric W. Biederman" <[email protected]>
Date: Mon, 24 Jan 2022 12:46:50 -0600
Subject: [PATCH] ucount: Make get_ucount a safe get_user replacement
When the ucount code was refactored to create get_ucount it was missed
that some of the contexts in which a rlimit is kept elevated can be
the only reference to the user/ucount in the system.
Ordinary ucount references exist in places that also have a reference
to the user namspace, but in POSIX message queues, the SysV shm code,
and the SIGPENDING code there is no independent user namespace
reference.
Inspection of the the user_namespace show no instance of circular
references between struct ucounts and the user_namespace. So
hold a reference from struct ucount to it's user_namespace to
resolve this problem.
Link: https://lore.kernel.org/lkml/[email protected]/
Reported-by: Qian Cai <[email protected]>
Reported-by: Mathias Krause <[email protected]>
Tested-by: Mathias Krause <[email protected]>
Reviewed-by: Mathias Krause <[email protected]>
Reviewed-by: Alexey Gladkov <[email protected]>
Fixes: d64696905554 ("Reimplement RLIMIT_SIGPENDING on top of ucounts")
Fixes: 6e52a9f0532f ("Reimplement RLIMIT_MSGQUEUE on top of ucounts")
Fixes: d7c9e99aee48 ("Reimplement RLIMIT_MEMLOCK on top of ucounts")
Cc: [email protected]
Signed-off-by: "Eric W. Biederman" <[email protected]>
---
kernel/ucount.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 7b32c356ebc5..65b597431c86 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -190,6 +190,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
kfree(new);
} else {
hlist_add_head(&new->node, hashent);
+ get_user_ns(new->ns);
spin_unlock_irq(&ucounts_lock);
return new;
}
@@ -210,6 +211,7 @@ void put_ucounts(struct ucounts *ucounts)
if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
hlist_del_init(&ucounts->node);
spin_unlock_irqrestore(&ucounts_lock, flags);
+ put_user_ns(ucounts->ns);
kfree(ucounts);
}
}
--
2.29.2
Eric
The pull request you sent on Fri, 28 Jan 2022 11:07:45 -0600:
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git ucount-rlimit-fixes-for-v5.17-rc2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/76fcbc9c7c57a5d44e7ca493d8f2f6eba3964f29
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Fri, Jan 28, 2022 at 11:07:45AM -0600, Eric W. Biederman wrote:
>
> Linus,
>
> Please pull the ucount-rlimit-fixes-for-v5.17-rc2 branch from the git tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git ucount-rlimit-fixes-for-v5.17-rc2
> HEAD: f9d87929d451d3e649699d0f1d74f71f77ad38f5 ucount: Make get_ucount a safe get_user replacement
>
>
> From: "Eric W. Biederman" <[email protected]>
> Date: Mon, 24 Jan 2022 12:46:50 -0600
> Subject: [PATCH] ucount: Make get_ucount a safe get_user replacement
>
> When the ucount code was refactored to create get_ucount it was missed
> that some of the contexts in which a rlimit is kept elevated can be
> the only reference to the user/ucount in the system.
>
> Ordinary ucount references exist in places that also have a reference
> to the user namspace, but in POSIX message queues, the SysV shm code,
> and the SIGPENDING code there is no independent user namespace
> reference.
>
> Inspection of the the user_namespace show no instance of circular
> references between struct ucounts and the user_namespace. So
> hold a reference from struct ucount to it's user_namespace to
> resolve this problem.
>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Reported-by: Qian Cai <[email protected]>
> Reported-by: Mathias Krause <[email protected]>
> Tested-by: Mathias Krause <[email protected]>
> Reviewed-by: Mathias Krause <[email protected]>
> Reviewed-by: Alexey Gladkov <[email protected]>
> Fixes: d64696905554 ("Reimplement RLIMIT_SIGPENDING on top of ucounts")
> Fixes: 6e52a9f0532f ("Reimplement RLIMIT_MSGQUEUE on top of ucounts")
> Fixes: d7c9e99aee48 ("Reimplement RLIMIT_MEMLOCK on top of ucounts")
> Cc: [email protected]
> Signed-off-by: "Eric W. Biederman" <[email protected]>
> ---
Hey,
Please ensure that next time a security issue is reported to
[email protected] for userns such as this UAF that the pull request
that gets sent to fix this issue Ccs the containers development list.
This whole ucount conversion has been quite problematic so far. And
that's not the problem, bugs happen. But fixes for bugs that were
reported as security issues should at least have visibility on the right
lists so people don't go and get pinged about them on Twitter [1].
A Cc for the oss-security list would've also sufficed where most of us
are subscribed. None of this is pleasant, I very much sympathise.
Thanks for fixing this, and thanks to Mathias for the report.
[1]: https://twitter.com/grsecurity/status/1487119590425600005
> kernel/ucount.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 7b32c356ebc5..65b597431c86 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -190,6 +190,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid)
> kfree(new);
> } else {
> hlist_add_head(&new->node, hashent);
> + get_user_ns(new->ns);
> spin_unlock_irq(&ucounts_lock);
> return new;
> }
> @@ -210,6 +211,7 @@ void put_ucounts(struct ucounts *ucounts)
> if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
> hlist_del_init(&ucounts->node);
> spin_unlock_irqrestore(&ucounts_lock, flags);
> + put_user_ns(ucounts->ns);
> kfree(ucounts);
> }
> }
> --
> 2.29.2
>
> eric
>