Re: [PATCHv3] gdb: linux-namespaces: enter user namespace when appropriate

[Tom de Vries <tdevries@suse.de>]

On 6/25/25 12:34, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> On 6/23/25 15:56, Andrew Burgess wrote:
>>> Andrew Burgess <aburgess@redhat.com> writes:
>>>
>>>> From: Benjamin Berg <benjamin@sipsolutions.net>
>>>>
>>>> In v2:
>>>>
>>>>     - Update the test to ignore a warning seen when running the test on
>>>>       a machine with libc debug information installed, but without the
>>>>       libc source being available, e.g.:
>>>>
>>>>       warning: 46     ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S: No such file or directory
>>>>
>>>>       This was causing some CI failures to be reported from Linaro.
>>>>
>>>>     - Rebased to current upstream/master.
>>>>
>>>> In v3:
>>>>
>>>>     - Same as V2, but fix the test pattern correctly this time.
>>>>
>>>> --
>>>>
>>>> The use of user namespaces is required for normal users to use mount
>>>> namespaces.  Consider trying this as an unprivileged user:
>>>>
>>>>     $ unshare --mount /bin/true
>>>>     unshare: unshare failed: Operation not permitted
>>>>
>>>> The problem here is that an unprivileged user doesn't have the
>>>> required permissions to create a new mount namespace.  If, instead, we
>>>> do this:
>>>>
>>>>     $ unshare --mount --map-root-user /bin/true
>>>>
>>>> then this will succeed.  The new option causes unshare to create a
>>>> user namespace in which the unprivileged user is mapped to UID/GID 0,
>>>> and so gains all privileges (inside the namespace), the user is then
>>>> able to create the mount namespace as required.
>>>>
>>>> So, how does this relate to GDB?
>>>>
>>>> When a user attaches to a process running in a separate mount
>>>> namespace, GDB makes use of a separate helper process (see
>>>> linux_mntns_get_helper in nat/linux-namespaces.c), which will then use
>>>> the `setns` function to enter (or try to enter) the mount namespace of
>>>> the process GDB is attaching too.  The helper process will then handle
>>>> file I/O requests received from GDB, and return the results back to
>>>> GDB, this allows GDB to access files within the mount namespace.
>>>>
>>>> The problem here is that, switching to a mount namespace requires that
>>>> a process hold CAP_SYS_CHROOT and CAP_SYS_ADMIN capabilities within
>>>> its user namespace (actually it's a little more complex, see 'man 2
>>>> setns').  Assuming GDB is running as an unprivileged user, then GDB
>>>> will not have the required permissions.
>>>>
>>>> However, if GDB enters the user namespace that the `unshare` process
>>>> created, then the current user will be mapped to UID/GID 0, and will
>>>> have the required permissions.
>>>>
>>>> And so, this patch extends linux_mntns_access_fs (in
>>>> nat/linux-namespace.c) to first try and switch to the user namespace
>>>> of the inferior before trying to switch to the mount namespace.  If
>>>> the inferior does have a user namespace, and does have elevated
>>>> privileges within that namespace, then this first switch by GDB will
>>>> mean that the second step, into the mount namespace, will succeed.
>>>>
>>>> If there is no user namespace, or the inferior doesn't have elevated
>>>> privileges within the user namespace, then the switch into the mount
>>>> namespace will fail, just as it currently does, and the user will need
>>>> to give elevated privileges to GDB via some other mechanism (e.g. run
>>>> as root).
>>>>
>>>> This work was originally posted here:
>>>>
>>>>     https://inbox.sourceware.org/gdb-patches/20230321120126.1418012-1-benjamin@sipsolutions.net
>>>>
>>>> I (Andrew Burgess) have made some cleanups to the code to comply with
>>>> GDB's coding standard, and the test is entirely mine.  This commit
>>>> message is also entirely mine -- the original message was very terse
>>>> and required the reader to understand how the various namespaces
>>>> work and interact.  The above is my attempt to document what I now
>>>> understand about the problem being fixed.
>>>>
>>>> I've left the original author in place as the core of the GDB change
>>>> itself is largely as originally presented, but any inaccuracies in the
>>>> commit message, or problems with the test, are all mine.
>>>>
>>>> Co-Authored-by: Andrew Burgess <aburgess@redhat.com>
>>>
>>> I've pushed this patch.
>>>
>>
>> The new test-case fails on arm32 (Linaro CI reported this, and I was
>> able to reproduce) due to insufficient permissions:
>> ...
>> (gdb) attach 184732
>> Attaching to process 184732
>> warning: process 184732 is a zombie - the process has already terminated
>> ptrace: Operation not permitted.
>> (gdb) FAIL: gdb.base/user-namespace-attach.exp: flags=--mount
>> --map-root-user: attach to inferior
>> ...
>>
>> In essence, the test-case assumes:
>> ...
>> $ unshare --mount --map-root-user /bin/true; echo $?
>> 0
>> ...
>> but we get instead:
>> ...
>> $ unshare --mount --map-root-user /bin/true; echo $?
>> unshare: unshare failed: Operation not permitted
>> 1
>> ...
>>
>> Filed here ( https://sourceware.org/bugzilla/show_bug.cgi?id=33108 ).
> 
> Hi!
> 
> Thanks for raising this issue.
> 
> What do you think to the patch below?
> 
> I've tested this by passing a bogus flag to `unshare`, e.g. "unshare
> -blahblah", which has the same effect of causing the `unshare` process
> to exit immediately with an exit code of 1.  I now see the test reported
> as unsupported.
> 

Hi Andrew,

thanks for picking this up.

FWIW, the problem with this solution is that a timeout now looks like 
unsupported.

More concretely, by doing this (on x86_64-linux, where the test-case 
passes for me):
...
diff --git a/gdb/testsuite/gdb.base/user-namespace-attach.exp 
b/gdb/testsuite/gdb.base/user
-namespace-attach.exp
index 01f3dae1693..c5ec5ef6369 100644
--- a/gdb/testsuite/gdb.base/user-namespace-attach.exp
+++ b/gdb/testsuite/gdb.base/user-namespace-attach.exp
@@ -66,6 +66,7 @@ proc run_test { flags } {
      }

      set inferior_pid [spawn_id_get_pid $inferior_spawn_id]
+    sleep 90

      clean_restart

...
I trigger the timeout of 60 seconds in the exec, and with your patch get:
...
		=== gdb Summary ===

# of unsupported tests		3
...
but without your patch I get:
...
# of unexpected failures	6
...

I don't think it's terribly important though.

You could try the approach I proposed in the PR, or you could pursue 
this one.

In the latter case, please add a comment that a timeout may trigger the 
same message.

Thanks,
- Tom

> Thanks,
> Andrew
> 
> ---
> 
> commit 1c61ee90c22666a6f33a474c27a901d03bbc5da9
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Wed Jun 25 11:24:30 2025 +0100
> 
>      gdb/testsuite: handle failure to start process to later attach to
>      
>      Commit:
>      
>        commit b23903836007d1acaf7f8c059ab000ee83fcebfa
>        Date:   Tue Mar 21 13:01:26 2023 +0100
>      
>            gdb: linux-namespaces: enter user namespace when appropriate
>      
>      added a new test gdb.base/user-namespace-attach.exp.  It has been
>      reported that this test will sometimes fail, like this:
>      
>        (gdb) attach 184732
>        Attaching to process 184732
>        warning: process 184732 is a zombie - the process has already terminated
>        ptrace: Operation not permitted.
>        (gdb) FAIL: gdb.base/user-namespace-attach.exp: flags=--mount --map-root-user: attach to inferior
>      
>      the test tries to run the 'unshare' application.  Sometimes though,
>      the application is present, but the set of flags used is not
>      supported (maybe due to restrictions on the local machine), so we see
>      behaviour like this:
>      
>        $ unshare --mount --map-root-user /bin/true; echo $?
>        unshare: unshare failed: Operation not permitted
>        1
>      
>      Handle this case by checking for the warning:
>      
>        warning: process 184732 is a zombie - the process has already terminated
>      
>      when GDB tries to attach.  If we see this warning then we assume the
>      'unshare' process failed to start correctly, in which case we report
>      the test as unsupported and perform an early return.
>      
>      Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33108
> 
> diff --git a/gdb/testsuite/gdb.base/user-namespace-attach.exp b/gdb/testsuite/gdb.base/user-namespace-attach.exp
> index 9936bb998eb..01f3dae1693 100644
> --- a/gdb/testsuite/gdb.base/user-namespace-attach.exp
> +++ b/gdb/testsuite/gdb.base/user-namespace-attach.exp
> @@ -69,12 +69,18 @@ proc run_test { flags } {
>   
>       clean_restart
>   
> +    set saw_failure_to_start false
>       set saw_bad_warning false
>       gdb_test_multiple "attach $inferior_pid" "attach to inferior" {
>   	-re "^attach $::decimal\r\n" {
>   	    exp_continue
>   	}
>   
> +	-re "^warning: process $::decimal is a zombie - the process has already terminated\r\n" {
> +	    set saw_failure_to_start true
> +	    exp_continue
> +	}
> +
>   	-re "^warning: \[^\r\n\]+: could not open as an executable file: \[^\r\n\]+\r\n" {
>   	    set saw_bad_warning true
>   	    exp_continue
> @@ -112,6 +118,11 @@ proc run_test { flags } {
>   	}
>   
>   	-re "^$::gdb_prompt $" {
> +	    if { $saw_failure_to_start } {
> +		unsupported "unshare process failed to start"
> +		return
> +	    }
> +
>   	    gdb_assert { !$saw_bad_warning } $gdb_test_name
>   	}
>   
>