From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sun, 1 Apr 2018 12:31:40 +0000 (+0200)
Subject: n-i-bz  Fix possible stack trashing by semctl syscall wrapping
X-Git-Tag: VALGRIND_3_14_0~132
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=54145019b045fffde625447b64f3a91f663de718;p=thirdparty%2Fvalgrind.git

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 faee5cd6c4..6577a5eed1 100644
--- 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 b0fbfd9393..7022316303 100644
--- 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 ae3ad7019a..707208040c 100644
--- 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 27db0713cf..b293d5c6e5 100644
--- 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);
 }