[PATCH 0/2] userns: clean up dead code on error paths

[PATCH 0/2] userns: clean up dead code on error paths

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Two small cleanups on the create_user_ns() and map_write() error
paths.  Both remove code that provably cannot have an effect; no
behavior change.

  user_namespace.c:164-167: the key_put() at fail_keyring has been
  dead since it was copy-pasted from free_user_ns() in 2016
  (dbec28460a89); persistent_keyring_register is only ever assigned
  lazily from key_get_persistent(), long after create_user_ns().

  user_namespace.c:1110-1112: the map clearing at the out: label of
  map_write() only ever zeroes an already-zero map; the destination
  map is untouched on every error path, and a second write to a
  mapped namespace is rejected before parsing.

Compile-tested and A/B-tested on next-20260827: identical behavior
on vanilla and patched kernels for single-write map failures,
recovery writes and 300 rounds of userns create/destroy.

Tao Cui (2):
  userns: remove dead key_put() on the create_user_ns() error path
  userns: don't clear the install target map on map_write() failure

 kernel/user_namespace.c | 6 ------
 1 file changed, 6 deletions(-)

-- 
2.43.0

[PATCH 1/2] userns: remove dead key_put() on the create_user_ns() error path

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

create_user_ns() jumps to fail_keyring when setup_userns_sysctls()
fails.  At that point ns was freshly allocated with
kmem_cache_zalloc() and ns->persistent_keyring_register is only ever
assigned later, lazily, from key_get_persistent()
(security/keys/persistent.c).  The key_put() therefore always
receives NULL.

Remove the dead call.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/user_namespace.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0bed462e9b2a..e9e04ce167df 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -162,9 +162,6 @@ int create_user_ns(struct cred *new)
 	ns_tree_add(ns);
 	return 0;
 fail_keyring:
-#ifdef CONFIG_PERSISTENT_KEYRINGS
-	key_put(ns->persistent_keyring_register);
-#endif
 	ns_common_free(ns);
 fail_free:
 	kmem_cache_free(user_ns_cachep, ns);
-- 
2.43.0

Re: [PATCH 1/2] userns: remove dead key_put() on the create_user_ns() error path

[Jan Kara]

On Fri 28-08-26 17:16:58, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> create_user_ns() jumps to fail_keyring when setup_userns_sysctls()
> fails.  At that point ns was freshly allocated with
> kmem_cache_zalloc() and ns->persistent_keyring_register is only ever
> assigned later, lazily, from key_get_persistent()
> (security/keys/persistent.c).  The key_put() therefore always
> receives NULL.
> 
> Remove the dead call.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>

Adding Eric to CC for verification but yes, that code seems to be dead
since its introduction by Eric 10 years ago. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  kernel/user_namespace.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0bed462e9b2a..e9e04ce167df 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -162,9 +162,6 @@ int create_user_ns(struct cred *new)
>  	ns_tree_add(ns);
>  	return 0;
>  fail_keyring:
> -#ifdef CONFIG_PERSISTENT_KEYRINGS
> -	key_put(ns->persistent_keyring_register);
> -#endif
>  	ns_common_free(ns);
>  fail_free:
>  	kmem_cache_free(user_ns_cachep, ns);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] userns: remove dead key_put() on the create_user_ns() error path

[Bradley Morgan]

On 28 August 2026 10:16:58 BST, Tao Cui <cui.tao@linux.dev> wrote:
>From: Tao Cui <cuitao@kylinos.cn>
>
>create_user_ns() jumps to fail_keyring when setup_userns_sysctls()
>fails.  At that point ns was freshly allocated with
>kmem_cache_zalloc() and ns->persistent_keyring_register is only ever
>assigned later, lazily, from key_get_persistent()
>(security/keys/persistent.c).  The key_put() therefore always
>receives NULL.
>
>Remove the dead call.
>

Agreed,

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

>Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>---
> kernel/user_namespace.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>index 0bed462e9b2a..e9e04ce167df 100644
>--- a/kernel/user_namespace.c
>+++ b/kernel/user_namespace.c
>@@ -162,9 +162,6 @@ int create_user_ns(struct cred *new)
> 	ns_tree_add(ns);
> 	return 0;
> fail_keyring:
>-#ifdef CONFIG_PERSISTENT_KEYRINGS
>-	key_put(ns->persistent_keyring_register);
>-#endif
> 	ns_common_free(ns);
> fail_free:
> 	kmem_cache_free(user_ns_cachep, ns);
>


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

[PATCH 2/2] userns: don't clear the install target map on map_write() failure

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

At the out: label of map_write() the destination map is also cleared
(map->forward = NULL; map->reverse = NULL; map->nr_extents = 0)
whenever the write failed and the extent arrays had to be freed.

However, the destination map is written by the successful install
block above, which has no failure exit; every error path reaches
out: without having touched it.  A second write to a mapped
namespace is also rejected with -EPERM before any parsing happens,
so the clearing can never roll back a previously installed map
either.

The three assignments just zero an already-zero map.  Remove them.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/user_namespace.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index e9e04ce167df..cef5e71779f6 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1104,9 +1104,6 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 	if (ret < 0 && new_map.nr_extents > UID_GID_MAP_MAX_BASE_EXTENTS) {
 		kfree(new_map.forward);
 		kfree(new_map.reverse);
-		map->forward = NULL;
-		map->reverse = NULL;
-		map->nr_extents = 0;
 	}
 
 	mutex_unlock(&userns_state_mutex);
-- 
2.43.0

Re: [PATCH 2/2] userns: don't clear the install target map on map_write() failure

[Jan Kara]

On Fri 28-08-26 17:16:59, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> At the out: label of map_write() the destination map is also cleared
> (map->forward = NULL; map->reverse = NULL; map->nr_extents = 0)
> whenever the write failed and the extent arrays had to be freed.
> 
> However, the destination map is written by the successful install
> block above, which has no failure exit; every error path reaches
> out: without having touched it.  A second write to a mapped
> namespace is also rejected with -EPERM before any parsing happens,
> so the clearing can never roll back a previously installed map
> either.
> 
> The three assignments just zero an already-zero map.  Remove them.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>

I agree this is a dead code but it kind of makes it easier to argue about
the correctness of the error handling branch. I guess I'll leave it to for
Christian to decide whether he wants to take this or not.

								Honza

> ---
>  kernel/user_namespace.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index e9e04ce167df..cef5e71779f6 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -1104,9 +1104,6 @@ static ssize_t map_write(struct file *file, const char __user *buf,
>  	if (ret < 0 && new_map.nr_extents > UID_GID_MAP_MAX_BASE_EXTENTS) {
>  		kfree(new_map.forward);
>  		kfree(new_map.reverse);
> -		map->forward = NULL;
> -		map->reverse = NULL;
> -		map->nr_extents = 0;
>  	}
>  
>  	mutex_unlock(&userns_state_mutex);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] userns: don't clear the install target map on map_write() failure

[Christian Brauner]

On Fri, Aug 28, 2026 at 11:51:10AM +0200, Jan Kara wrote:
> On Fri 28-08-26 17:16:59, Tao Cui wrote:
> > From: Tao Cui <cuitao@kylinos.cn>
> > 
> > At the out: label of map_write() the destination map is also cleared
> > (map->forward = NULL; map->reverse = NULL; map->nr_extents = 0)
> > whenever the write failed and the extent arrays had to be freed.
> > 
> > However, the destination map is written by the successful install
> > block above, which has no failure exit; every error path reaches
> > out: without having touched it.  A second write to a mapped
> > namespace is also rejected with -EPERM before any parsing happens,
> > so the clearing can never roll back a previously installed map
> > either.
> > 
> > The three assignments just zero an already-zero map.  Remove them.
> > 
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> 
> I agree this is a dead code but it kind of makes it easier to argue about
> the correctness of the error handling branch. I guess I'll leave it to for
> Christian to decide whether he wants to take this or not.

Nah, let's leave this as is. :)

Re: [PATCH 2/2] userns: don't clear the install target map on map_write() failure

[Tao Cui]

在 2026/8/31 18:08, Christian Brauner 写道:
> On Fri, Aug 28, 2026 at 11:51:10AM +0200, Jan Kara wrote:
>> On Fri 28-08-26 17:16:59, Tao Cui wrote:
>>> From: Tao Cui <cuitao@kylinos.cn>
>>>
>>> At the out: label of map_write() the destination map is also cleared
>>> (map->forward = NULL; map->reverse = NULL; map->nr_extents = 0)
>>> whenever the write failed and the extent arrays had to be freed.
>>>
>>> However, the destination map is written by the successful install
>>> block above, which has no failure exit; every error path reaches
>>> out: without having touched it.  A second write to a mapped
>>> namespace is also rejected with -EPERM before any parsing happens,
>>> so the clearing can never roll back a previously installed map
>>> either.
>>>
>>> The three assignments just zero an already-zero map.  Remove them.
>>>
>>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>>
>> I agree this is a dead code but it kind of makes it easier to argue about
>> the correctness of the error handling branch. I guess I'll leave it to for
>> Christian to decide whether he wants to take this or not.
> 
> Nah, let's leave this as is. :)

Understood, that's a fair point.  I'm fine dropping this one.

Thanks for the review, Jan.  Thanks for picking up 1/2, Christian.
And thanks for the Reviewed-by tag, Bradley.

--
Tao

Re: [PATCH 2/2] userns: don't clear the install target map on map_write() failure

[Bradley Morgan]

On 28 August 2026 10:16:59 BST, Tao Cui <cui.tao@linux.dev> wrote:
>From: Tao Cui <cuitao@kylinos.cn>
>
>At the out: label of map_write() the destination map is also cleared
>(map->forward = NULL; map->reverse = NULL; map->nr_extents = 0)
>whenever the write failed and the extent arrays had to be freed.
>
>However, the destination map is written by the successful install
>block above, which has no failure exit; every error path reaches
>out: without having touched it.  A second write to a mapped
>namespace is also rejected with -EPERM before any parsing happens,
>so the clearing can never roll back a previously installed map
>either.
>
>The three assignments just zero an already-zero map.  Remove them.
>

Thinking about NAKing, but I have comments.

>Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>---
> kernel/user_namespace.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>index e9e04ce167df..cef5e71779f6 100644
>--- a/kernel/user_namespace.c
>+++ b/kernel/user_namespace.c
>@@ -1104,9 +1104,6 @@ static ssize_t map_write(struct file *file, const char __user *buf,
> 	if (ret < 0 && new_map.nr_extents > UID_GID_MAP_MAX_BASE_EXTENTS) {
> 		kfree(new_map.forward);
> 		kfree(new_map.reverse);
>-		map->forward = NULL;
>-		map->reverse = NULL;
>-		map->nr_extents = 0;

Yup, that's dead code, and I would add a tag here, but the thing is, this
code is a defensive pattern, so if some bloke decided "hmmmm let's add a
failure path", then that would be stupid if this code was gone.

If I was the maintainer, I wouldn't take it.


> 	}
> 
> 	mutex_unlock(&userns_state_mutex);
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

Re: (subset) [PATCH 0/2] userns: clean up dead code on error paths

[Christian Brauner]

On Fri, 28 Aug 2026 17:16:57 +0800, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> Two small cleanups on the create_user_ns() and map_write() error
> paths.  Both remove code that provably cannot have an effect; no
> behavior change.
> 
>   user_namespace.c:164-167: the key_put() at fail_keyring has been
>   dead since it was copy-pasted from free_user_ns() in 2016
>   (dbec28460a89); persistent_keyring_register is only ever assigned
>   lazily from key_get_persistent(), long after create_user_ns().
> 
> [...]

Applied to the vfs-7.4.namespace branch of the vfs/vfs.git tree.
Patches in the vfs-7.4.namespace 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: master

[1/2] userns: remove dead key_put() on the create_user_ns() error path
      https://git.kernel.org/vfs/vfs/c/fec2c3c0744b