Re: [PATCH 03/11] vfs: Fix race condition on get_userns_fd()

[Christian Brauner <brauner@kernel.org>]

On Tue, Mar 07, 2023 at 12:44:59PM +0100, Rodrigo Campos wrote:
> Talking with Christian Brauner about a different problem, he mentioned
> that technically this race condition exists and we should fix it.
> 
> The race is that when we clone, we call a function that just returns
> while at the same time we try to get the userns via /proc/pid/ns/user.
> The thing is that, while the pid needs to be reaped, Christian said that
> the userns file cease to exist as soon as the program finishes.

See exit_task_namespaces() in kernel/exit.c:do_exit().

> 
> So, let's make the function never return, so we always can get the
> userns. We are already sending a SIGKILL to this pid, so nothing else
> remaining to not leak the process.
> 
> Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
> ---
>  src/vfs/utils.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git src/vfs/utils.c src/vfs/utils.c
> index ea7536c1..67779e83 100644
> --- src/vfs/utils.c
> +++ src/vfs/utils.c
> @@ -58,9 +58,10 @@ pid_t do_clone(int (*fn)(void *), void *arg, int flags)
>  #endif
>  }
>  
> -static int get_userns_fd_cb(void *data)
> +__attribute__((noreturn)) static int get_userns_fd_cb(void *data)
>  {
> -	return 0;
> +	for (;;)
> +		pause();

Should this add a _exit(0)? It's pretty odd otherwise. And do we need
noreturn?