FreeBSD ioctl: add handling for FIODGNAME

Up til now FreeBSD ioctl coverage was conspicuous by its absence
relying entirely on the generic wrapper. Time to start improving
that.

Example provided by Kyle Evans.

diff --git a/.gitignore b/.gitignore
index 88a6522605dbaf92bc19cb82fac03f0802b8d451..7ad5cf912b031ace692baedf5e73bd64dfc713a4 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -1395,6 +1395,7 @@
 /memcheck/tests/freebsd/linkat
 /memcheck/tests/freebsd/memalign
 /memcheck/tests/freebsd/misc
+/memcheck/tests/freebsd/openpty
 /memcheck/tests/freebsd/pdfork_pdkill
 /memcheck/tests/freebsd/realpathat
 /memcheck/tests/freebsd/revoke

diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c
index bfb1cf876e8cc798e999751f240a0e5f0577fde3..d7e63ef4bbff7978ea869b0048e9f42a9824b7d3 100644 (file)
--- a/coregrind/m_syswrap/syswrap-freebsd.c
+++ b/coregrind/m_syswrap/syswrap-freebsd.c
@@ -974,53 +974,23 @@ PRE(sys_setlogin)
 // int ioctl(int fd, unsigned long request, ...);
 PRE(sys_ioctl)
 {
-   UInt dir  = _VKI_IOC_DIR(ARG2);
-   UInt size = _VKI_IOC_SIZE(ARG2);
    *flags |= SfMayBlock;
    // @todo PJF presumably the presence of ARG3 depends on ARG2
    PRINT("sys_ioctl ( %" FMT_REGWORD "u, 0x%" FMT_REGWORD "x, %#" FMT_REGWORD "x )",ARG1,ARG2,ARG3);
    PRE_REG_READ3(int, "ioctl",
                  int, fd, unsigned long, request, unsigned long, arg);
 
-   /* On FreeBSD, ALL ioctl's are IOR/IOW encoded.  Just use the default decoder */
-   if (SimHintiS(SimHint_lax_ioctls, VG_(clo_sim_hints))) {
-      /*
-      * Be very lax about ioctl handling; the only
-      * assumption is that the size is correct. Doesn't
-      * require the full buffer to be initialized when
-      * writing.  Without this, using some device
-      * drivers with a large number of strange ioctl
-      * commands becomes very tiresome.
-      */
-   } else if (dir == _VKI_IOC_NONE && size > 0) {
-      static UWord unknown_ioctl[10];
-      static Int moans = sizeof(unknown_ioctl) / sizeof(unknown_ioctl[0]);
-      if (moans > 0 && !VG_(clo_xml)) {
-         /* Check if have not already moaned for this request. */
-         UInt i;
-         for (i = 0; i < sizeof(unknown_ioctl)/sizeof(unknown_ioctl[0]); i++) {
-            if (unknown_ioctl[i] == ARG2) {
-               break;
-            }
-            if (unknown_ioctl[i] == 0) {
-               unknown_ioctl[i] = ARG2;
-               moans--;
-               VG_(umsg)("Warning: noted but unhandled ioctl 0x%lx"
-                         " with no direction hints.\n", ARG2);
-               VG_(umsg)("   This could cause spurious value errors to appear.\n");
-               VG_(umsg)("   See README_MISSING_SYSCALL_OR_IOCTL for "
-                         "guidance on writing a proper wrapper.\n" );
-               return;
-            }
-         }
-      }
-   } else {
-      if ((dir & _VKI_IOC_WRITE) && size > 0) {
-         PRE_MEM_READ( "ioctl(generic)", ARG3, size);
-      }
-      if ((dir & _VKI_IOC_READ) && size > 0) {
-         PRE_MEM_WRITE( "ioctl(generic)", ARG3, size);
-      }
+   switch (ARG2) {
+   case VKI_FIODGNAME: {
+      struct vki_fiodgname_arg* data = (struct vki_fiodgname_arg*)(Addr)ARG3;
+      PRE_FIELD_READ("ioctl(FIODGNAME).len", data->len);
+      PRE_FIELD_READ("ioctl(FIODGNAME).buf", data->buf);
+      PRE_MEM_WRITE("ioctl(FIODGNAME).buf", (Addr)data->buf, data->len);
+      break;
+   }
+   default:
+      ML_(PRE_unknown_ioctl)(tid, ARG2, ARG3);
+      break;
    }
 
    // The block below is from Ryan Stone
@@ -1087,12 +1057,15 @@ PRE(sys_ioctl)
 
 POST(sys_ioctl)
 {
-   UInt dir  = _VKI_IOC_DIR(ARG2);
-   UInt size = _VKI_IOC_SIZE(ARG2);
-   vg_assert(SUCCESS);
-   if (size > 0 && (dir & _VKI_IOC_READ)
-         && RES == 0 && ARG3 != (Addr)NULL) {
-      POST_MEM_WRITE(ARG3, size);
+   switch (ARG2) {
+   case VKI_FIODGNAME: {
+      struct vki_fiodgname_arg* data = (struct vki_fiodgname_arg*)(Addr)ARG3;
+      POST_MEM_WRITE((Addr)data->buf, data->len);
+      break;
+   }
+   default:
+      ML_(POST_unknown_ioctl)(tid, RES, ARG2, ARG3);
+      break;
    }
 
 #if 0

diff --git a/include/vki/vki-freebsd.h b/include/vki/vki-freebsd.h
index d19c8de0184dc81b5bc7618b682de36b04118083..abbdf49c276057f025a2978d7d647fa601c39b8d 100644 (file)
--- a/include/vki/vki-freebsd.h
+++ b/include/vki/vki-freebsd.h
@@ -1086,6 +1086,11 @@ extern unsigned int __vki_invalid_size_argument_for_IOC;
 #define VKI_FIOASYNC _VKI_IOW('f', 125, int)
 #define VKI_FIOSETOWN   _VKI_IOW('f', 124, int)
 #define VKI_FIOGETOWN   _VKI_IOW('f', 123, int)
+struct vki_fiodgname_arg {
+   int     len;
+   void    *buf;
+};
+#define VKI_FIODGNAME   _VKI_IOW('f', 120, struct vki_fiodgname_arg) /* get dev. name */
 
 // See syswrap-freebsd.c PRE/POST(sys_ioctl)
 #if 0

diff --git a/memcheck/tests/freebsd/Makefile.am b/memcheck/tests/freebsd/Makefile.am
index 466799cefdcce45395acd247ca83b22edf84bb4f..d9efd74a990a94b581129332f7db7e0c38d41b3d 100644 (file)
--- a/memcheck/tests/freebsd/Makefile.am
+++ b/memcheck/tests/freebsd/Makefile.am
@@ -76,6 +76,8 @@ EXTRA_DIST = \
        memalign_supp.supp \
        misc.vgtest \
        misc.stderr.exp \
+       openpty.vgtest \
+       openpty.stderr.exp \
        pdfork_pdkill.vgtest \
        pdfork_pdkill.stderr.exp \
        realpathat.vgtest \
@@ -144,6 +146,7 @@ check_PROGRAMS = \
        file_locking_wait6 \
        get_set_context get_set_login getfh \
        kqueue linkat memalign misc \
+       openpty \
        pdfork_pdkill getfsstat inlinfo inlinfo_nested.so \
        revoke scalar \
        scalar_fork scalar_thr_exit scalar_abort2 scalar_pdfork \
@@ -200,3 +203,5 @@ errno_aligned_allocs_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_NON_POWER_OF_TWO_ALIGNMENT
 extattr_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_UNUSED_BUT_SET_VARIABLE@
 
 memalign_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_NON_POWER_OF_TWO_ALIGNMENT@
+
+openpty_LDFLAGS = ${AM_LDFLAGS} -lutil

diff --git a/memcheck/tests/freebsd/openpty.c b/memcheck/tests/freebsd/openpty.c
new file mode 100644 (file)
index 0000000..bfba8ce
--- /dev/null
+++ b/memcheck/tests/freebsd/openpty.c
@@ -0,0 +1,51 @@
+/* Build with cc -lutil valgrind-ptsname.c */
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <unistd.h>
+#include <termios.h>
+
+#include <libutil.h>
+
+/*
+ * ==18203== Syscall param ioctl(generic) points to uninitialised byte(s)
+ * ==18203==    at 0x49AAE7A: ioctl (in /lib/libc.so.7)
+ * ==18203==    by 0x490C116: fdevname_r (in /lib/libc.so.7)
+ * ==18203==    by 0x49E1567: ptsname (in /lib/libc.so.7)
+ * ==18203==    by 0x486B5E2: openpty (in /lib/libutil.so.9)
+ * ==18203==    by 0x2016E6: main (in /tmp/a.out)
+ * ==18203==  Address 0x1ffc000a74 is on thread 1's stack
+ * ==18203==  in frame #1, created by fdevname_r (???:)
+ */
+
+#if 0
+/* Relevant bit from lib/libc/gen/fdevname.c */
+char *
+fdevname_r(int fd, char *buf, int len)
+{
+   struct fiodgname_arg fgn;
+
+   /*
+    * buf here points to a `static` buffer in ptsname.c, these are the only
+    * two members of fiodgname_arg.
+    */
+   fgn.buf = buf;
+   fgn.len = len;
+
+   if (_ioctl(fd, FIODGNAME, &fgn) == -1)
+      return (NULL);
+   return (buf);
+}
+#endif
+
+int
+main()
+{
+   int master, slave;
+
+   (void)openpty(&master, &slave, NULL, NULL, NULL);
+
+   return (0);
+}
+

diff --git a/memcheck/tests/freebsd/openpty.stderr.exp b/memcheck/tests/freebsd/openpty.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/memcheck/tests/freebsd/openpty.vgtest b/memcheck/tests/freebsd/openpty.vgtest
new file mode 100644 (file)
index 0000000..0568f0a
--- /dev/null
+++ b/memcheck/tests/freebsd/openpty.vgtest
@@ -0,0 +1,2 @@
+prog: openpty
+vgopts: -q