[8/9,v2] Implement vFile:setfs in gdbserver
Commit Message
This commit implements the "vFile:setfs" packet in gdbserver.
gdb/gdbserver/ChangeLog:
* target.h (struct target_ops) <multifs_open>: New field.
<multifs_unlink>: Likewise.
<multifs_readlink>: Likewise.
* linux-low.c (nat/linux-namespaces.h): New include.
(linux_target_ops): Initialize the_target->multifs_open,
the_target->multifs_unlink and the_target->multifs_readlink.
* hostio.c (hostio_fs_pid): New static variable.
(handle_setfs): New function.
(handle_open): Use the_target->multifs_open as appropriate.
(handle_unlink): Use the_target->multifs_unlink as appropriate.
(handle_readlink): Use the_target->multifs_readlink as
appropriate.
(handle_vFile): Handle vFile:setfs packets.
---
gdb/gdbserver/ChangeLog | 16 +++++++++++
gdb/gdbserver/hostio.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
gdb/gdbserver/linux-low.c | 4 +++
gdb/gdbserver/target.h | 21 +++++++++++++++
4 files changed, 100 insertions(+), 3 deletions(-)
Comments
On 04/30/2015 01:05 PM, Gary Benson wrote:
> This commit implements the "vFile:setfs" packet in gdbserver.
>
> gdb/gdbserver/ChangeLog:
>
> * target.h (struct target_ops) <multifs_open>: New field.
> <multifs_unlink>: Likewise.
> <multifs_readlink>: Likewise.
> * linux-low.c (nat/linux-namespaces.h): New include.
> (linux_target_ops): Initialize the_target->multifs_open,
> the_target->multifs_unlink and the_target->multifs_readlink.
> * hostio.c (hostio_fs_pid): New static variable.
> (handle_setfs): New function.
> (handle_open): Use the_target->multifs_open as appropriate.
> (handle_unlink): Use the_target->multifs_unlink as appropriate.
> (handle_readlink): Use the_target->multifs_readlink as
> appropriate.
> (handle_vFile): Handle vFile:setfs packets.
> ---
> gdb/gdbserver/ChangeLog | 16 +++++++++++
> gdb/gdbserver/hostio.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
> gdb/gdbserver/linux-low.c | 4 +++
> gdb/gdbserver/target.h | 21 +++++++++++++++
> 4 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
> index 9e858d9..10cff5a 100644
> --- a/gdb/gdbserver/hostio.c
> +++ b/gdb/gdbserver/hostio.c
> @@ -243,6 +243,47 @@ hostio_reply_with_data (char *own_buf, char *buffer, int len,
> return input_index;
> }
>
> +/* Process ID of inferior whose filesystem hostio functions
> + that take FILENAME arguments will use. Zero means to use
> + our own filesystem. */
> +
> +static int hostio_fs_pid = 0;
This should be cleared if GDB reconnects.
> +
> +/* Handle a "vFile:setfs:" packet. */
> +
> +static void
> +handle_setfs (char *own_buf)
> +{
> + char *p;
> + int pid;
> +
> + /* If the target doesn't have any of the in-filesystem-of methods
> + then there's no point in GDB sending "vFile:setfs:" packets. We
> + reply with an empty packet (i.e. we pretend we don't understand
> + "vFile:setfs:") and that should stop GDB sending any more. */
> + if (the_target->multifs_open == NULL
> + && the_target->multifs_unlink == NULL
> + && the_target->multifs_readlink == NULL)
> + {
> + own_buf[0] = '\0';
> + return;
> + }
For the setns/ENOSYS case, how about somehow probing whether setns
actually works here (with a new method, or probing one of the existing
ones with getpid()) and return empty packet, instead of ending up with
open failing with ENOSYS later on?
Some comment for the native side, actually.
Thanks,
Pedro Alves
Pedro Alves wrote:
> On 04/30/2015 01:05 PM, Gary Benson wrote:
> > This commit implements the "vFile:setfs" packet in gdbserver.
> >
> > gdb/gdbserver/ChangeLog:
> >
> > * target.h (struct target_ops) <multifs_open>: New field.
> > <multifs_unlink>: Likewise.
> > <multifs_readlink>: Likewise.
> > * linux-low.c (nat/linux-namespaces.h): New include.
> > (linux_target_ops): Initialize the_target->multifs_open,
> > the_target->multifs_unlink and the_target->multifs_readlink.
> > * hostio.c (hostio_fs_pid): New static variable.
> > (handle_setfs): New function.
> > (handle_open): Use the_target->multifs_open as appropriate.
> > (handle_unlink): Use the_target->multifs_unlink as appropriate.
> > (handle_readlink): Use the_target->multifs_readlink as
> > appropriate.
> > (handle_vFile): Handle vFile:setfs packets.
> > ---
> > gdb/gdbserver/ChangeLog | 16 +++++++++++
> > gdb/gdbserver/hostio.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
> > gdb/gdbserver/linux-low.c | 4 +++
> > gdb/gdbserver/target.h | 21 +++++++++++++++
> > 4 files changed, 100 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
> > index 9e858d9..10cff5a 100644
> > --- a/gdb/gdbserver/hostio.c
> > +++ b/gdb/gdbserver/hostio.c
> > @@ -243,6 +243,47 @@ hostio_reply_with_data (char *own_buf, char *buffer, int len,
> > return input_index;
> > }
> >
> > +/* Process ID of inferior whose filesystem hostio functions
> > + that take FILENAME arguments will use. Zero means to use
> > + our own filesystem. */
> > +
> > +static int hostio_fs_pid = 0;
>
>
> This should be cleared if GDB reconnects.
Ok, I added a hostio_handle_new_gdb_connection function to do this
that's called just after target_handle_new_gdb_connection in server.c.
> > +/* Handle a "vFile:setfs:" packet. */
> > +
> > +static void
> > +handle_setfs (char *own_buf)
> > +{
> > + char *p;
> > + int pid;
> > +
> > + /* If the target doesn't have any of the in-filesystem-of methods
> > + then there's no point in GDB sending "vFile:setfs:" packets. We
> > + reply with an empty packet (i.e. we pretend we don't understand
> > + "vFile:setfs:") and that should stop GDB sending any more. */
> > + if (the_target->multifs_open == NULL
> > + && the_target->multifs_unlink == NULL
> > + && the_target->multifs_readlink == NULL)
> > + {
> > + own_buf[0] = '\0';
> > + return;
> > + }
>
> For the setns/ENOSYS case, how about somehow probing whether setns
> actually works here (with a new method, or probing one of the
> existing ones with getpid()) and return empty packet, instead of
> ending up with open failing with ENOSYS later on?
Probing and then telling GDB we don't understand "vFile:setfs:" isn't
that good of an idea. If we do that with an inferior in another mount
namespace then the end result is that gdbserver will access the wrong
filesystem. You might get "file not found", or you might get an open
filehandle on another file (i.e. the file with the same name but in
gdbserver's filesystem). If the inferior's filesystem is different
from gdbserver and gdbserver cannot access that filesystem then the
only correct response is to fail.
I've updated linux_mntns_access_fs to translate ENOSYS from setns
into ENOTSUP, so neither native nor gdbserver will ever get ENOSYS
from this code. From [1] ENOTSUP seems to be the perfect code for
linux_mntns_{open_cloexec,unlink,readlink} to be returning:
A function returns this error when certain parameter values are
valid, but the functionality they request is not available. This can
mean that the function does not implement a particular command or
option value or flag bit at all. ... If the entire function is not
available at all in the implementation, it returns ENOSYS instead.
Thanks for hassling me about this, it took me a while to understand
what you were saying.
Cheers,
Gary
--
[1] http://lkml.iu.edu/hypermail/linux/kernel/0208.0/0121.html
On 06/09/2015 03:11 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> For the setns/ENOSYS case, how about somehow probing whether setns
>> actually works here (with a new method, or probing one of the
>> existing ones with getpid()) and return empty packet, instead of
>> ending up with open failing with ENOSYS later on?
>
> Probing and then telling GDB we don't understand "vFile:setfs:" isn't
> that good of an idea. If we do that with an inferior in another mount
> namespace then the end result is that gdbserver will access the wrong
> filesystem. You might get "file not found", or you might get an open
> filehandle on another file (i.e. the file with the same name but in
> gdbserver's filesystem). If the inferior's filesystem is different
> from gdbserver and gdbserver cannot access that filesystem then the
> only correct response is to fail.
If the running system does not support "setfs", because it fails setns
with ENOSYS, meaning, the system call isn't implemented at all, how
can one end up in the situation that an inferior on the same kernel
is running in a different filesystem namespace?
>
> I've updated linux_mntns_access_fs to translate ENOSYS from setns
> into ENOTSUP, so neither native nor gdbserver will ever get ENOSYS
> from this code. From [1] ENOTSUP seems to be the perfect code for
> linux_mntns_{open_cloexec,unlink,readlink} to be returning:
Great, thanks.
> Thanks for hassling me about this, it took me a while to understand
> what you were saying.
Thanks for the patience and following through.
Thanks,
Pedro Alves
Pedro Alves wrote:
> On 06/09/2015 03:11 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > For the setns/ENOSYS case, how about somehow probing whether
> > > setns actually works here (with a new method, or probing one
> > > of the existing ones with getpid()) and return empty packet,
> > > instead of ending up with open failing with ENOSYS later on?
> >
> > Probing and then telling GDB we don't understand "vFile:setfs:"
> > isn't that good of an idea. If we do that with an inferior in
> > another mount namespace then the end result is that gdbserver will
> > access the wrong filesystem. You might get "file not found", or
> > you might get an open filehandle on another file (i.e. the file
> > with the same name but in gdbserver's filesystem). If the
> > inferior's filesystem is different from gdbserver and gdbserver
> > cannot access that filesystem then the only correct response is
> > to fail.
>
> If the running system does not support "setfs", because it fails
> setns with ENOSYS, meaning, the system call isn't implemented at
> all, how can one end up in the situation that an inferior on the
> same kernel is running in a different filesystem namespace?
If the running kernel has namespaces support but GDB and/or glibc
was built on a kernel without. It's an edge case but it's possible.
Cheers,
Gary
Gary Benson wrote:
> Pedro Alves wrote:
> > On 06/09/2015 03:11 PM, Gary Benson wrote:
> > > Pedro Alves wrote:
> > > > For the setns/ENOSYS case, how about somehow probing whether
> > > > setns actually works here (with a new method, or probing one
> > > > of the existing ones with getpid()) and return empty packet,
> > > > instead of ending up with open failing with ENOSYS later on?
> > >
> > > Probing and then telling GDB we don't understand "vFile:setfs:"
> > > isn't that good of an idea. If we do that with an inferior in
> > > another mount namespace then the end result is that gdbserver will
> > > access the wrong filesystem. You might get "file not found", or
> > > you might get an open filehandle on another file (i.e. the file
> > > with the same name but in gdbserver's filesystem). If the
> > > inferior's filesystem is different from gdbserver and gdbserver
> > > cannot access that filesystem then the only correct response is
> > > to fail.
> >
> > If the running system does not support "setfs", because it fails
> > setns with ENOSYS, meaning, the system call isn't implemented at
> > all, how can one end up in the situation that an inferior on the
> > same kernel is running in a different filesystem namespace?
>
> If the running kernel has namespaces support but GDB and/or glibc
> was built on a kernel without. It's an edge case but it's possible.
Note that GDB/gdbserver will not attempt to call setns unless the
inferior is actually in a different mount namespace. If you're
running without namespaces support it won't even start the helper
let alone try to setns. Same goes for if you have namespaces
support but the inferior is in GDB/gdbserver's mount namespace.
Nothing calls setns unless it is 100% necessary.
Cheers,
Gary
On 06/10/2015 10:40 AM, Gary Benson wrote:
> Gary Benson wrote:
>> Pedro Alves wrote:
>>> On 06/09/2015 03:11 PM, Gary Benson wrote:
>>>> Pedro Alves wrote:
>>>>> For the setns/ENOSYS case, how about somehow probing whether
>>>>> setns actually works here (with a new method, or probing one
>>>>> of the existing ones with getpid()) and return empty packet,
>>>>> instead of ending up with open failing with ENOSYS later on?
>>>>
>>>> Probing and then telling GDB we don't understand "vFile:setfs:"
>>>> isn't that good of an idea. If we do that with an inferior in
>>>> another mount namespace then the end result is that gdbserver will
>>>> access the wrong filesystem. You might get "file not found", or
>>>> you might get an open filehandle on another file (i.e. the file
>>>> with the same name but in gdbserver's filesystem). If the
>>>> inferior's filesystem is different from gdbserver and gdbserver
>>>> cannot access that filesystem then the only correct response is
>>>> to fail.
>>>
>>> If the running system does not support "setfs", because it fails
>>> setns with ENOSYS, meaning, the system call isn't implemented at
>>> all, how can one end up in the situation that an inferior on the
>>> same kernel is running in a different filesystem namespace?
>>
>> If the running kernel has namespaces support but GDB and/or glibc
>> was built on a kernel without. It's an edge case but it's possible.
I see.
>
> Note that GDB/gdbserver will not attempt to call setns unless the
> inferior is actually in a different mount namespace. If you're
> running without namespaces support it won't even start the helper
> let alone try to setns. Same goes for if you have namespaces
> support but the inferior is in GDB/gdbserver's mount namespace.
> Nothing calls setns unless it is 100% necessary.
To probe for setns I was thinking would be done by on
gdb/gdbserver's pid, by gdb/gdbserver directly.
But I agree, when the kernel supports namespaces, and we can
tell the inferior is in a different namespace, but then setns
doesn't work, it's better to error out than disabling the setfs
packet. So I'm happy with the errno translation alone.
Thanks,
Pedro Alves
@@ -243,6 +243,47 @@ hostio_reply_with_data (char *own_buf, char *buffer, int len,
return input_index;
}
+/* Process ID of inferior whose filesystem hostio functions
+ that take FILENAME arguments will use. Zero means to use
+ our own filesystem. */
+
+static int hostio_fs_pid = 0;
+
+/* Handle a "vFile:setfs:" packet. */
+
+static void
+handle_setfs (char *own_buf)
+{
+ char *p;
+ int pid;
+
+ /* If the target doesn't have any of the in-filesystem-of methods
+ then there's no point in GDB sending "vFile:setfs:" packets. We
+ reply with an empty packet (i.e. we pretend we don't understand
+ "vFile:setfs:") and that should stop GDB sending any more. */
+ if (the_target->multifs_open == NULL
+ && the_target->multifs_unlink == NULL
+ && the_target->multifs_readlink == NULL)
+ {
+ own_buf[0] = '\0';
+ return;
+ }
+
+ p = own_buf + strlen ("vFile:setfs:");
+
+ if (require_int (&p, &pid)
+ || pid < 0
+ || require_end (p))
+ {
+ hostio_packet_error (own_buf);
+ return;
+ }
+
+ hostio_fs_pid = pid;
+
+ hostio_reply (own_buf, 0);
+}
+
static void
handle_open (char *own_buf)
{
@@ -267,7 +308,11 @@ handle_open (char *own_buf)
/* We do not need to convert MODE, since the fileio protocol
uses the standard values. */
- fd = open (filename, flags, mode);
+ if (hostio_fs_pid != 0 && the_target->multifs_open != NULL)
+ fd = the_target->multifs_open (hostio_fs_pid, filename,
+ flags, mode);
+ else
+ fd = open (filename, flags, mode);
if (fd == -1)
{
@@ -471,7 +516,10 @@ handle_unlink (char *own_buf)
return;
}
- ret = unlink (filename);
+ if (hostio_fs_pid != 0 && the_target->multifs_unlink != NULL)
+ ret = the_target->multifs_unlink (hostio_fs_pid, filename);
+ else
+ ret = unlink (filename);
if (ret == -1)
{
@@ -498,7 +546,13 @@ handle_readlink (char *own_buf, int *new_packet_len)
return;
}
- ret = readlink (filename, linkname, sizeof (linkname) - 1);
+ if (hostio_fs_pid != 0 && the_target->multifs_readlink != NULL)
+ ret = the_target->multifs_readlink (hostio_fs_pid, filename,
+ linkname,
+ sizeof (linkname) - 1);
+ else
+ ret = readlink (filename, linkname, sizeof (linkname) - 1);
+
if (ret == -1)
{
hostio_error (own_buf);
@@ -532,6 +586,8 @@ handle_vFile (char *own_buf, int packet_len, int *new_packet_len)
handle_unlink (own_buf);
else if (startswith (own_buf, "vFile:readlink:"))
handle_readlink (own_buf, new_packet_len);
+ else if (startswith (own_buf, "vFile:setfs:"))
+ handle_setfs (own_buf);
else
return 0;
@@ -51,6 +51,7 @@
definition of elf_fpregset_t. */
#include <elf.h>
#endif
+#include "nat/linux-namespaces.h"
#ifndef SPUFS_MAGIC
#define SPUFS_MAGIC 0x23c9b64e
@@ -6434,6 +6435,9 @@ static struct target_ops linux_target_ops = {
#endif
linux_supports_range_stepping,
linux_proc_pid_to_exec_file,
+ linux_mntns_open_cloexec,
+ linux_mntns_unlink,
+ linux_mntns_readlink,
};
static void
@@ -402,6 +402,27 @@ struct target_ops
string should be copied into a buffer by the client if the string
will not be immediately used, or if it must persist. */
char *(*pid_to_exec_file) (int pid);
+
+ /* Multiple-filesystem-aware open. Like open(2), but operating in
+ the filesystem as it appears to process PID. Systems where all
+ processes share a common filesystem should set this to NULL.
+ If NULL, the caller should fall back to open(2). */
+ int (*multifs_open) (int pid, const char *filename,
+ int flags, mode_t mode);
+
+ /* Multiple-filesystem-aware unlink. Like unlink(2), but operates
+ in the filesystem as it appears to process PID. Systems where
+ all processes share a common filesystem should set this to NULL.
+ If NULL, the caller should fall back to unlink(2). */
+ int (*multifs_unlink) (int pid, const char *filename);
+
+ /* Multiple-filesystem-aware readlink. Like readlink(2), but
+ operating in the filesystem as it appears to process PID.
+ Systems where all processes share a common filesystem should
+ set this to NULL. If NULL, the caller should fall back to
+ readlink(2). */
+ ssize_t (*multifs_readlink) (int pid, const char *filename,
+ char *buf, size_t bufsiz);
};
extern struct target_ops *the_target;