| Message ID | 20220131110650.186464-1-cascardo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,OEM-5.14/Jammy] ucount: Make get_ucount a safe get_user replacement | expand |
On 31.1.2022 13.06, Thadeu Lima de Souza Cascardo wrote: > From: "Eric W. Biederman" <ebiederm@xmission.com> > > 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 i's user_namespace to > resolve this problem. > > Link: https://lore.kernel.org/lkml/YZV7Z+yXbsx9p3JN@fixkernel.com/ > Reported-by: Qian Cai <quic_qiancai@quicinc.com> > Reported-by: Mathias Krause <minipli@grsecurity.net> > Tested-by: Mathias Krause <minipli@grsecurity.net> > Reviewed-by: Mathias Krause <minipli@grsecurity.net> > Reviewed-by: Alexey Gladkov <legion@kernel.org> > 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: stable@vger.kernel.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > (cherry picked from commit f9d87929d451d3e649699d0f1d74f71f77ad38f5) > CVE-2022-24122 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > kernel/ucount.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 16feb710ee63..804f64799fc1 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -184,6 +184,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; > } > @@ -204,6 +205,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); > } > } applied to oem-5.14, thanks
On Mon, Jan 31, 2022 at 08:06:50AM -0300, Thadeu Lima de Souza Cascardo wrote: > From: "Eric W. Biederman" <ebiederm@xmission.com> > > 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 i's user_namespace to > resolve this problem. > > Link: https://lore.kernel.org/lkml/YZV7Z+yXbsx9p3JN@fixkernel.com/ > Reported-by: Qian Cai <quic_qiancai@quicinc.com> > Reported-by: Mathias Krause <minipli@grsecurity.net> > Tested-by: Mathias Krause <minipli@grsecurity.net> > Reviewed-by: Mathias Krause <minipli@grsecurity.net> > Reviewed-by: Alexey Gladkov <legion@kernel.org> > 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: stable@vger.kernel.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > (cherry picked from commit f9d87929d451d3e649699d0f1d74f71f77ad38f5) > CVE-2022-24122 > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- Applied to jammy/linux, thanks! -Andrea > kernel/ucount.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 16feb710ee63..804f64799fc1 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -184,6 +184,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; > } > @@ -204,6 +205,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.32.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/kernel/ucount.c b/kernel/ucount.c index 16feb710ee63..804f64799fc1 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -184,6 +184,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; } @@ -204,6 +205,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); } }