Re: [PATCH 1/2][PR GDB/32956] gdb: implement linux namespace support for fileio_stat

[Fabian Kilger <kilger@sec.in.tum.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 8264 bytes --]

While looking at it, I've noticed all implementations of stat functions
actually use lstat and not stat. This maybe should be modified in the
namespace case as well and I'll be changing this for v2. However, I
could not directly find a rationale behind why *_stat functions call
lstat instead. Possibly, it might make sense renaming the
target_fileio_stat to target_fileio_lstat as well, though this would be
an independent change.

Best,
Fabian

On 5/24/25 21:43, Fabian Kilger wrote:
> Hi Andrew,
> 
> you're right, I'm directly using GDB and wasn't aware of that gdbserver
> module. I'll be adding it to the patch for v2.
> 
> Best,
> Fabian
> 
> On 5/24/25 12:46, Andrew Burgess wrote:
>> Fabian Kilger <kilger@sec.in.tum.de> writes:
>>
>>> The new algorithm to look for a build-id-based debug file
>>> (introduced by commit 22836ca88591ac7efacf06d5b6db191763fd8aba)
>>> makes use of fileio_stat. As fileio_stat was not supported by
>>> linux-namespace.c, all stat calls would be performed on the host
>>> and not inside the namespace
>>>
>>> ---
>>>  gdb/linux-nat.c            | 14 ++++++++
>>>  gdb/linux-nat.h            |  3 ++
>>>  gdb/nat/linux-namespaces.c | 71 ++++++++++++++++++++++++++++++++++++++
>>>  gdb/nat/linux-namespaces.h |  6 ++++
>>
>> Thinking about this some more, I realised that this is missing the
>> gdbserver related changes.
>>
>> If you search in the gdbserver/ directory for how multifs_readlink is
>> used then this will give a pretty good idea for how a new multifs_stat
>> should be added.
>>
>> For manual testing things will be pretty similar to testing GDB.  On the
>> same host as your container, but outside of the container, start
>> gdbserver:
>>
>>   $ gdbserver --multi --once :54321
>>
>> Then on the same host, start GDB, and within GDB:
>>
>>   (gdb) target extended-remote :54321
>>   (gdb) attach PID
>>
>> I'm assuming that currently you are just attaching directly from GDB to
>> a process within the container?
>>
>> Thanks,
>> Andrew
>>
>>>  4 files changed, 94 insertions(+)
>>>
>>> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
>>> index 3f252370c7b..478a7977c4d 100644
>>> --- a/gdb/linux-nat.c
>>> +++ b/gdb/linux-nat.c
>>> @@ -4585,6 +4585,20 @@ linux_nat_target::fileio_open (struct inferior *inf, const char *filename,
>>>    return fd;
>>>  }
>>>  
>>> +/* Implementation of to_fileio_stat.  */
>>> +int
>>> +linux_nat_target::fileio_stat (struct inferior *inf, const char *filename,
>>> +			       struct stat *sb, fileio_error *target_errno)
>>> +{
>>> +  int r = linux_mntns_stat (linux_nat_fileio_pid_of (inf),
>>> +			    filename, sb);
>>> +
>>> +  if (r == -1)
>>> +    *target_errno = host_to_fileio_error (errno);
>>> +
>>> +  return r;
>>> +}
>>> +
>>>  /* Implementation of to_fileio_readlink.  */
>>>  
>>>  std::optional<std::string>
>>> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
>>> index b630b858e34..42d1ec142b3 100644
>>> --- a/gdb/linux-nat.h
>>> +++ b/gdb/linux-nat.h
>>> @@ -108,6 +108,9 @@ class linux_nat_target : public inf_ptrace_target
>>>  		     const char *filename,
>>>  		     fileio_error *target_errno) override;
>>>  
>>> +  int fileio_stat (struct inferior *inf, const char *filename,
>>> +		   struct stat *sb, fileio_error *target_errno) override;
>>> +
>>>    int fileio_unlink (struct inferior *inf,
>>>  		     const char *filename,
>>>  		     fileio_error *target_errno) override;
>>> diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
>>> index 19a05eec905..aa74e9df950 100644
>>> --- a/gdb/nat/linux-namespaces.c
>>> +++ b/gdb/nat/linux-namespaces.c
>>> @@ -233,6 +233,12 @@ enum mnsh_msg_type
>>>         MNSH_RET_INT.  */
>>>      MNSH_REQ_SETNS,
>>>  
>>> +    /* A request that the helper call stat.  The single
>>> +       argument (the filename) should be passed in BUF, and
>>> +       should include a terminating NUL character.  The helper
>>> +       should respond with a MNSH_RET_INTSTR.  */
>>> +    MNSH_REQ_STAT,
>>> +
>>>      /* A request that the helper call open.  Arguments should
>>>         be passed in BUF, INT1 and INT2.  The filename (in BUF)
>>>         should include a terminating NUL character.  The helper
>>> @@ -283,6 +289,10 @@ mnsh_debug_print_message (enum mnsh_msg_type type,
>>>        debug_printf ("ERROR");
>>>        break;
>>>  
>>> +    case MNSH_REQ_STAT:
>>> +      debug_printf ("STAT");
>>> +      break;
>>> +
>>>      case MNSH_REQ_SETNS:
>>>        debug_printf ("SETNS");
>>>        break;
>>> @@ -514,6 +524,20 @@ mnsh_handle_setns (int sock, int fd, int nstype)
>>>    return mnsh_return_int (sock, result, errno);
>>>  }
>>>  
>>> +
>>> +/* Handle a MNSH_REQ_STAT message.  Must be async-signal-safe.  */
>>> +
>>> +static ssize_t
>>> +mnsh_handle_stat(int sock, const char *filename)
>>> +{
>>> +  struct stat sb;
>>> +  int stat_ok = stat(filename, &sb);
>>> +
>>> +  return mnsh_return_intstr(sock, stat_ok, &sb,
>>> +			    stat_ok == -1 ? 0 : sizeof (sb),
>>> +			    errno);
>>> +}
>>> +
>>>  /* Handle a MNSH_REQ_OPEN message.  Must be async-signal-safe.  */
>>>  
>>>  static ssize_t
>>> @@ -574,6 +598,11 @@ mnsh_main (int sock)
>>>  		response = mnsh_handle_setns (sock, fd, int1);
>>>  	      break;
>>>  
>>> +	    case MNSH_REQ_STAT:
>>> +	      if (size > 0 && buf[size - 1] == '\0')
>>> +		response = mnsh_handle_stat(sock, buf);
>>> +	      break;
>>> +
>>>  	    case MNSH_REQ_OPEN:
>>>  	      if (size > 0 && buf[size - 1] == '\0')
>>>  		response = mnsh_handle_open (sock, buf, int1, int2);
>>> @@ -765,6 +794,10 @@ mnsh_maybe_mourn_peer (void)
>>>    mnsh_send_message (helper->sock, MNSH_REQ_OPEN, -1, flags, mode, \
>>>  		     filename, strlen (filename) + 1)
>>>  
>>> +#define mnsh_send_stat(helper, filename) \
>>> +  mnsh_send_message (helper->sock, MNSH_REQ_STAT, -1, 0, 0, \
>>> +		     filename, strlen (filename) + 1)
>>> +
>>>  #define mnsh_send_unlink(helper, filename) \
>>>    mnsh_send_message (helper->sock, MNSH_REQ_UNLINK, -1, 0, 0, \
>>>  		     filename, strlen (filename) + 1)
>>> @@ -945,6 +978,44 @@ linux_mntns_access_fs (pid_t pid)
>>>    return MNSH_FS_HELPER;
>>>  }
>>>  
>>> +
>>> +/* See nat/linux-namespaces.h.  */
>>> +int
>>> +linux_mntns_stat (pid_t pid, const char *filename,
>>> +		  struct stat *sb)
>>> +{
>>> +  enum mnsh_fs_code access = linux_mntns_access_fs (pid);
>>> +  struct linux_mnsh *helper;
>>> +  int stat_ok, error;
>>> +  ssize_t size;
>>> +
>>> +  if (access == MNSH_FS_ERROR)
>>> +    return -1;
>>> +
>>> +  if (access == MNSH_FS_DIRECT)
>>> +    return stat(filename, sb);
>>> +
>>> +  gdb_assert (access == MNSH_FS_HELPER);
>>> +
>>> +  helper = linux_mntns_get_helper ();
>>> +
>>> +  size = mnsh_send_stat (helper, filename);
>>> +  if (size < 0)
>>> +    return -1;
>>> +
>>> +  size = mnsh_recv_intstr (helper, &stat_ok, &error, sb, sizeof (*sb));
>>> +
>>> +  if (size < 0)
>>> +    {
>>> +      stat_ok = -1;
>>> +      errno = error;
>>> +    }
>>> +  else
>>> +    gdb_assert (stat_ok == -1 || size == sizeof (*sb));
>>> +
>>> +  return stat_ok;
>>> +}
>>> +
>>>  /* See nat/linux-namespaces.h.  */
>>>  
>>>  int
>>> diff --git a/gdb/nat/linux-namespaces.h b/gdb/nat/linux-namespaces.h
>>> index 4327292950b..825cb27eb2d 100644
>>> --- a/gdb/nat/linux-namespaces.h
>>> +++ b/gdb/nat/linux-namespaces.h
>>> @@ -58,6 +58,12 @@ enum linux_ns_type
>>>  
>>>  extern int linux_ns_same (pid_t pid, enum linux_ns_type type);
>>>  
>>> +/* Like stat(2), but in the mount namespace of process
>>> +   PID.  */
>>> +
>>> +extern int linux_mntns_stat (pid_t pid, const char *filename,
>>> +			     struct stat *sb);
>>> +
>>>  /* Like gdb_open_cloexec, but in the mount namespace of process
>>>     PID.  */
>>>  
>>> -- 
>>> 2.49.0
>>
> 

-- 
Fabian Kilger, M.Sc.
Wissenschaftlicher Mitarbeiter

Technische Universität München
TUM School of Computation, Information and Technology
Chair of IT Security

Boltzmannstraße 3
85748 Garching (bei München)

Tel. +49 (0)89 289-18587
Fax +49 (0)89 289-18579

kilger@sec.in.tum.de
www.sec.in.tum.de

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 653 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]