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

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On 3/7/23 17:46, Christian Brauner wrote:
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().

Cool, thanks! Added that instead, then :)


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@xxxxxxxxxxx>
---
  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?

Agree, let's do that and remove the attribute.



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux