n-i-bz Fix possible stack trashing by semctl syscall wrapping

The modified test none/tests/sem crashes with a SEGV when valgrind is compiled
with lto on various amd64 platforms (debian/gcc 6.3, RHEL7/gcc 6.4,
Ubuntu/gcc 7.2)

The problem is that the vki_semid_ds buf is not what is expected by the kernel:
the kernel expects a bigger structure vki_semid64_ds (at least on
these platforms).
Getting the sem_nsems seems to work by chance, as sem_nsems is at
the same offset in both vki_semid_ds and vki_semid64_ds.
However, e.g. the ctime was not set properly after syscall return,
and 2 words after sem_nsems were set to 0 by the kernel, causing
the SEGV, as a spilled register became 0.

Fix consists in using the 64 bit version for __NR_semctl.
Tested on debian/amd64 and s390x.

diff --git a/NEWS b/NEWS
index faee5cd6c4d2a6f3d27fbf66751615249c68f9bf..6577a5eed19fc9fc06604f4437b655f591ea45b7 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -106,6 +106,7 @@ n-i-bz  Fix missing workq_ops operations (macOS)
 n-i-bz  fix bug in strspn replacement
 n-i-bz  Add support for the Linux BLKFLSBUF ioctl
 n-i-bz  Add support for the Linux BLKREPORTZONE and BLKRESETZONE ioctls
+n-i-bz  Fix possible stack trashing by semctl syscall wrapping
 
 Release 3.13.0 (15 June 2017)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index b0fbfd9393ca3d27069c56de1c41bac211aa4989..702231630310585ac1029843d2cdea560f9b6702 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -1790,30 +1790,36 @@ ML_(generic_PRE_sys_semtimedop) ( ThreadId tid,
 static
 UInt get_sem_count( Int semid )
 {
-   struct vki_semid_ds buf;
    union vki_semun arg;
    SysRes res;
 
-   /* Doesn't actually seem to be necessary, but gcc-4.4.0 20081017
-      (experimental) otherwise complains that the use in the return
-      statement below is uninitialised. */
-   buf.sem_nsems = 0;
-
-   arg.buf = &buf;
-
 #  if defined(__NR_semctl)
+   struct vki_semid64_ds buf;
+   arg.buf64 = &buf;
    res = VG_(do_syscall4)(__NR_semctl, semid, 0, VKI_IPC_STAT, *(UWord *)&arg);
+   if (sr_isError(res))
+      return 0;
+
+   return buf.sem_nsems;
 #  elif defined(__NR_semsys) /* Solaris */
+   struct vki_semid_ds buf;
+   arg.buf = &buf;
    res = VG_(do_syscall5)(__NR_semsys, VKI_SEMCTL, semid, 0, VKI_IPC_STAT,
                           *(UWord *)&arg);
+   if (sr_isError(res))
+      return 0;
+
+   return buf.sem_nsems;
 #  else
+   struct vki_semid_ds buf;
+   arg.buf = &buf;
    res = VG_(do_syscall5)(__NR_ipc, 3 /* IPCOP_semctl */, semid, 0,
                           VKI_IPC_STAT, (UWord)&arg);
-#  endif
    if (sr_isError(res))
       return 0;
 
    return buf.sem_nsems;
+#  endif
 }
 
 void

diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h
index ae3ad7019a82ef6a4e144aa3fe07fd0e7de9daf7..707208040c7248e0a2ffd630ff62e70bfb03daba 100644 (file)
--- a/include/vki/vki-linux.h
+++ b/include/vki/vki-linux.h
@@ -1205,6 +1205,7 @@ struct vki_sembuf {
 union vki_semun {
        int val;                        /* value for SETVAL */
        struct vki_semid_ds __user *buf;        /* buffer for IPC_STAT & IPC_SET */
+       struct vki_semid64_ds __user *buf64;    /* buffer for IPC_STAT & IPC_SET */
        unsigned short __user *array;   /* array for GETALL & SETALL */
        struct vki_seminfo __user *__buf;       /* buffer for IPC_INFO */
        void __user *__pad;

diff --git a/none/tests/sem.c b/none/tests/sem.c
index 27db0713cfa65d98e8a122e569987810cdfd2315..b293d5c6e5939d49862353a6d50c462a80cd7e65 100644 (file)
--- a/none/tests/sem.c
+++ b/none/tests/sem.c
@@ -8,6 +8,59 @@
 #include <sys/sem.h>
 #include <time.h>
 #include <unistd.h>
+
+void semctl_test (int trace, const char *fname)
+{
+  key_t key;
+  int semid;
+  int nr_of_readers;
+  int ret;
+
+  union semun {
+     int              val;    /* Value for SETVAL */
+     struct semid_ds *buf;    /* Buffer for IPC_STAT, IPC_SET */
+     unsigned short  *array;  /* Array for GETALL, SETALL */
+     struct seminfo  *__buf;  /* Buffer for IPC_INFO
+                                 (Linux-specific) */
+  } u;
+
+  struct semid_ds ds;
+
+  key = ftok (fname, 1);
+  if (key == -1)
+    perror ("ftok");
+  nr_of_readers = 4;
+
+  semid = semget (key, 2 * nr_of_readers, IPC_CREAT + 0660);
+  if (semid == -1) {
+    perror ("semget");
+  }
+  if (trace)
+     printf("semid %d\n", semid);
+
+  u.buf = &ds;
+  ret = semctl (semid, 0, IPC_STAT, u);
+  if (ret == -1)
+     perror("semctl IPC_STAT");
+  if (trace)
+     printf("semid %d sem_nsems %d\n", semid, (int) ds.sem_nsems);
+
+  {
+    unsigned short semarray[2 * nr_of_readers];
+    for (int count = 0; count < nr_of_readers; count++) {
+        semarray[2 * count] = 0;
+        semarray[2 * count + 1] = 1000;
+      }
+    ret = semctl (semid, 0, SETALL, semarray);
+    if (ret == -1)
+      perror ("semctl SETALL");
+  }
+
+  ret = semctl (semid, 0, IPC_RMID);
+  if (ret == -1)
+    perror ("semctl IPC_RMID");
+}
+
 int main(int argc, char **argv)
 {
    int semid;
@@ -98,6 +151,7 @@ int main(int argc, char **argv)
       perror("semctl(IPC_RMID)");
       exit(1);
    }
-         
+
+   semctl_test(argc > 1, argv[0]);
    exit(0);
 }