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@xxxxxxxxx> wrote:
>From: Tao Cui <cuitao@xxxxxxxxxx>
>
>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@xxxxxxxxxx>
>---
> 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@xxxxxxxxx/