diff mbox

[OB] Don't assume File-I/O mode bits match the host's format

Message ID 1433853129-9470-1-git-send-email-gbenson@redhat.com
State New
Headers show

Commit Message

Gary Benson June 9, 2015, 12:32 p.m. UTC
Hi all,

inf_child_fileio_open and its gdbserver equivalent both assume
that the mode_t bits defined in gdb/fileio.h are the same as
those used by the open system call, but there is no mechanism
to ensure this is the case.  This commit adds a conversion
function to handle systems where the File-I/O definitions do
not align with the host's, as discussed here:

  https://sourceware.org/ml/gdb-patches/2015-05/msg00653.html

Built and regtested on RHEL 6.6 x86_64, pushed as obvious.

Thanks,
Gary

--
gdb/ChangeLog:

	* common/fileio.h (fileio_to_host_mode): New declaration.
	* common/fileio.c (fileio_to_host_mode): New Function.
	* inf-child.c (inf_child_fileio_open): Process mode argument
	with fileio_to_host_mode.

gdb/gdbserver/ChangeLog:

	* hostio.c (handle_open): Process mode argument with
	fileio_to_host_mode.
---
 gdb/ChangeLog           |    7 ++++++
 gdb/common/fileio.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/fileio.h     |    5 ++++
 gdb/gdbserver/ChangeLog |    5 ++++
 gdb/gdbserver/hostio.c  |    8 ++++--
 gdb/inf-child.c         |    8 +++---
 6 files changed, 75 insertions(+), 7 deletions(-)

Comments

Andreas Schwab June 9, 2015, 12:43 p.m. UTC | #1
Gary Benson <gbenson@redhat.com> writes:

> +  if (fileio_mode & ~FILEIO_S_SUPPORTED)
> +    return -1;

> +  if (fileio_mode & FILEIO_S_IFCHR)
> +    mode |= S_IFCHR;

FILEIO_S_SUPPORTED doesn't include FILEIO_S_IFCHR, so this is never
true.

Andreas.
Gary Benson June 9, 2015, 1:27 p.m. UTC | #2
Andreas Schwab wrote:
> Gary Benson <gbenson@redhat.com> writes:
> 
> > +  if (fileio_mode & ~FILEIO_S_SUPPORTED)
> > +    return -1;
> 
> > +  if (fileio_mode & FILEIO_S_IFCHR)
> > +    mode |= S_IFCHR;
> 
> FILEIO_S_SUPPORTED doesn't include FILEIO_S_IFCHR, so this is never
> true.

Good spot!

It seems to me FILEIO_S_SUPPORTED should include FILEIO_S_IFCHR, would
anybody object if I made that change?

Thanks,
Gary
diff mbox

Patch

diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c
index ef00ea8..d219f30 100644
--- a/gdb/common/fileio.c
+++ b/gdb/common/fileio.c
@@ -109,6 +109,55 @@  fileio_to_host_openflags (int fileio_open_flags, int *open_flags_p)
   return 0;
 }
 
+/* See fileio.h.  */
+
+int
+fileio_to_host_mode (int fileio_mode, mode_t *mode_p)
+{
+  mode_t mode = 0;
+
+  if (fileio_mode & ~FILEIO_S_SUPPORTED)
+    return -1;
+
+  if (fileio_mode & FILEIO_S_IFREG)
+    mode |= S_IFREG;
+  if (fileio_mode & FILEIO_S_IFDIR)
+    mode |= S_IFDIR;
+  if (fileio_mode & FILEIO_S_IFCHR)
+    mode |= S_IFCHR;
+  if (fileio_mode & FILEIO_S_IRUSR)
+    mode |= S_IRUSR;
+  if (fileio_mode & FILEIO_S_IWUSR)
+    mode |= S_IWUSR;
+  if (fileio_mode & FILEIO_S_IXUSR)
+    mode |= S_IXUSR;
+#ifdef S_IRGRP
+  if (fileio_mode & FILEIO_S_IRGRP)
+    mode |= S_IRGRP;
+#endif
+#ifdef S_IWGRP
+  if (fileio_mode & FILEIO_S_IWGRP)
+    mode |= S_IWGRP;
+#endif
+#ifdef S_IXGRP
+  if (fileio_mode & FILEIO_S_IXGRP)
+    mode |= S_IXGRP;
+#endif
+  if (fileio_mode & FILEIO_S_IROTH)
+    mode |= S_IROTH;
+#ifdef S_IWOTH
+  if (fileio_mode & FILEIO_S_IWOTH)
+    mode |= S_IWOTH;
+#endif
+#ifdef S_IXOTH
+  if (fileio_mode & FILEIO_S_IXOTH)
+    mode |= S_IXOTH;
+#endif
+
+  *mode_p = mode;
+  return 0;
+}
+
 /* Convert a host-format mode_t into a bitmask of File-I/O flags.  */
 
 static LONGEST
diff --git a/gdb/common/fileio.h b/gdb/common/fileio.h
index b0f27ab..88f96cf 100644
--- a/gdb/common/fileio.h
+++ b/gdb/common/fileio.h
@@ -32,6 +32,11 @@  extern int host_to_fileio_error (int error);
 
 extern int fileio_to_host_openflags (int fflags, int *flags);
 
+/* Convert File-I/O mode FMODE to host format, storing
+   the result in *MODE.  Return 0 on success, -1 on error.  */
+
+extern int fileio_to_host_mode (int fmode, mode_t *mode);
+
 /* Pack a host-format integer into a byte buffer in big-endian
    format.  BYTES specifies the size of the integer to pack in
    bytes.  */
diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index 9e858d9..811f958 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -248,7 +248,8 @@  handle_open (char *own_buf)
 {
   char filename[HOSTIO_PATH_MAX];
   char *p;
-  int fileio_flags, mode, flags, fd;
+  int fileio_flags, fileio_mode, flags, fd;
+  mode_t mode;
   struct fd_list *new_fd;
 
   p = own_buf + strlen ("vFile:open:");
@@ -257,9 +258,10 @@  handle_open (char *own_buf)
       || require_comma (&p)
       || require_int (&p, &fileio_flags)
       || require_comma (&p)
-      || require_int (&p, &mode)
+      || require_int (&p, &fileio_mode)
       || require_end (p)
-      || fileio_to_host_openflags (fileio_flags, &flags))
+      || fileio_to_host_openflags (fileio_flags, &flags)
+      || fileio_to_host_mode (fileio_mode, &mode))
     {
       hostio_packet_error (own_buf);
       return;
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 084dfa1..013bf0c 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -213,17 +213,17 @@  inf_child_fileio_open (struct target_ops *self,
 		       int *target_errno)
 {
   int nat_flags;
+  mode_t nat_mode;
   int fd;
 
-  if (fileio_to_host_openflags (flags, &nat_flags) == -1)
+  if (fileio_to_host_openflags (flags, &nat_flags) == -1
+      || fileio_to_host_mode (mode, &nat_mode) == -1)
     {
       *target_errno = FILEIO_EINVAL;
       return -1;
     }
 
-  /* We do not need to convert MODE, since the fileio protocol uses
-     the standard values.  */
-  fd = gdb_open_cloexec (filename, nat_flags, mode);
+  fd = gdb_open_cloexec (filename, nat_flags, nat_mode);
   if (fd == -1)
     *target_errno = host_to_fileio_error (errno);