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@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.
> 
> Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>

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@xxxxxxxx>
SUSE Labs, CR