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: 7539 bytes --]

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 --]