* [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path
@ 2025-11-18 6:45 Pavel Tikhomirov
2025-11-18 10:30 ` Alexey Gladkov
2025-11-18 21:34 ` Liam R. Howlett
0 siblings, 2 replies; 3+ messages in thread
From: Pavel Tikhomirov @ 2025-11-18 6:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Kees Cook, Eric W. Biederman,
Alexey Gladkov, linux-kernel, linux-mm, Pavel Tikhomirov
If unshare_nsproxy_namespaces() successfully creates the new_nsproxy,
but then set_cred_ucounts() fails, on its error path there is no cleanup
for new_nsproxy, so it is leaked. Let's fix that by freeing new_nsproxy
if it's not NULL on this error path.
Fixes: 905ae01c4ae2a ("Add a reference to ucounts for each cred")
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
kernel/fork.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 3da0f08615a95..6f7332e3e0c8c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -3133,8 +3133,11 @@ int ksys_unshare(unsigned long unshare_flags)
if (new_cred) {
err = set_cred_ucounts(new_cred);
- if (err)
+ if (err) {
+ if (new_nsproxy)
+ free_nsproxy(new_nsproxy);
goto bad_unshare_cleanup_cred;
+ }
}
if (new_fs || new_fd || do_sysvsem || new_cred || new_nsproxy) {
--
2.51.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path
2025-11-18 6:45 [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path Pavel Tikhomirov
@ 2025-11-18 10:30 ` Alexey Gladkov
2025-11-18 21:34 ` Liam R. Howlett
1 sibling, 0 replies; 3+ messages in thread
From: Alexey Gladkov @ 2025-11-18 10:30 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Kees Cook, Eric W. Biederman,
linux-kernel, linux-mm, stable
On Tue, Nov 18, 2025 at 02:45:50PM +0800, Pavel Tikhomirov wrote:
> If unshare_nsproxy_namespaces() successfully creates the new_nsproxy,
> but then set_cred_ucounts() fails, on its error path there is no cleanup
> for new_nsproxy, so it is leaked. Let's fix that by freeing new_nsproxy
> if it's not NULL on this error path.
>
> Fixes: 905ae01c4ae2a ("Add a reference to ucounts for each cred")
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Cc: stable@vger.kernel.org
Acked-by: Alexey Gladkov <legion@kernel.org>
> ---
> kernel/fork.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 3da0f08615a95..6f7332e3e0c8c 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -3133,8 +3133,11 @@ int ksys_unshare(unsigned long unshare_flags)
>
> if (new_cred) {
> err = set_cred_ucounts(new_cred);
> - if (err)
> + if (err) {
> + if (new_nsproxy)
> + free_nsproxy(new_nsproxy);
> goto bad_unshare_cleanup_cred;
> + }
> }
>
> if (new_fs || new_fd || do_sysvsem || new_cred || new_nsproxy) {
> --
> 2.51.1
>
--
Rgrds, legion
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path
2025-11-18 6:45 [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path Pavel Tikhomirov
2025-11-18 10:30 ` Alexey Gladkov
@ 2025-11-18 21:34 ` Liam R. Howlett
1 sibling, 0 replies; 3+ messages in thread
From: Liam R. Howlett @ 2025-11-18 21:34 UTC (permalink / raw)
To: Pavel Tikhomirov
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Kees Cook, Eric W. Biederman,
Alexey Gladkov, linux-kernel, linux-mm
* Pavel Tikhomirov <ptikhomirov@virtuozzo.com> [251118 01:46]:
> If unshare_nsproxy_namespaces() successfully creates the new_nsproxy,
> but then set_cred_ucounts() fails, on its error path there is no cleanup
> for new_nsproxy, so it is leaked. Let's fix that by freeing new_nsproxy
> if it's not NULL on this error path.
new_nsproxy may be set to an error pointer, but that case is handled
earlier in this function. That's a pretty subtle detail.
unshare_nsproxy_namespaces() should probably set it to NULL if it's an
error.. everywhere else looks fine.
This fix looks good!
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
>
> Fixes: 905ae01c4ae2a ("Add a reference to ucounts for each cred")
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
> kernel/fork.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 3da0f08615a95..6f7332e3e0c8c 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -3133,8 +3133,11 @@ int ksys_unshare(unsigned long unshare_flags)
>
> if (new_cred) {
> err = set_cred_ucounts(new_cred);
> - if (err)
> + if (err) {
> + if (new_nsproxy)
> + free_nsproxy(new_nsproxy);
> goto bad_unshare_cleanup_cred;
> + }
> }
>
> if (new_fs || new_fd || do_sysvsem || new_cred || new_nsproxy) {
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-18 21:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-18 6:45 [PATCH] unshare: Fix nsproxy leak on set_cred_ucounts() error path Pavel Tikhomirov
2025-11-18 10:30 ` Alexey Gladkov
2025-11-18 21:34 ` Liam R. Howlett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox