Bug 472219 - Syscall param ppoll(ufds.events) points to uninitialised byte(s)

Add checks that (p)poll fd is not negative. If it is negative, don't check
the events field.

diff --git a/.gitignore b/.gitignore
index 9e16ac126dca14c852e9baa67d9492b3163eb30a..6538eb718b45ee2a462d23564862d217e1f1fe20 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -845,6 +845,7 @@
 /memcheck/tests/bug287260
 /memcheck/tests/bug340392
 /memcheck/tests/bug464969_d_demangle
+/memcheck/tests/bug472219
 /memcheck/tests/calloc-overflow
 /memcheck/tests/cdebug_zlib
 /memcheck/tests/cdebug_zlib_gnu

diff --git a/NEWS b/NEWS
index 783612fbb90857e5a070310cea0c5fba3c4a225d..867d2f0f43cac77092fc616645db65b40be068dc 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -45,6 +45,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
         Assertion 'resolved' failed
 470830  Don't print actions vgdb me ... continue for vgdb --multi mode
 470978  s390x: Valgrind cannot start qemu-kvm when "sysctl vm.allocate_pgste=0"
+472219  Syscall param ppoll(ufds.events) points to uninitialised byte(s)
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX

diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c
index 6b9f3d2109816e57465fcfcc742bd2b9a6630029..9af37cfb8313a0456a1a885e450af97e3cfd1f23 100644 (file)
--- a/coregrind/m_syswrap/syswrap-freebsd.c
+++ b/coregrind/m_syswrap/syswrap-freebsd.c
@@ -6124,15 +6124,15 @@ PRE(sys_ppoll)
                  struct vki_pollfd *, fds, unsigned int, nfds,
                  struct vki_timespec *, timeout, vki_sigset_t *, newsigmask);
 
-   if (ML_(safe_to_deref)(fds, ARG2*sizeof(struct vki_pollfd))) {
-      for (i = 0; i < ARG2; i++) {
-         PRE_MEM_READ( "ppoll(fds.fd)",
-                       (Addr)(&fds[i].fd), sizeof(fds[i].fd) );
+   for (i = 0; i < ARG2; i++) {
+      PRE_MEM_READ( "ppoll(fds.fd)",
+                    (Addr)(&fds[i].fd), sizeof(fds[i].fd) );
+      if (ML_(safe_to_deref)(&fds[i].fd, sizeof(fds[i].fd)) && fds[i].fd >= 0) {
          PRE_MEM_READ( "ppoll(fds.events)",
                        (Addr)(&fds[i].events), sizeof(fds[i].events) );
-         PRE_MEM_WRITE( "ppoll(fds.revents)",
-                        (Addr)(&fds[i].revents), sizeof(fds[i].revents) );
       }
+      PRE_MEM_WRITE( "ppoll(fds.revents)",
+                     (Addr)(&fds[i].revents), sizeof(fds[i].revents) );
    }
 
    if (ARG3) {

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index efdae60e10320cbcccd86415fabf6ef8fd7abf57..ed9d14685fe16bc935d8d91ef4edecd692a6c192 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -4339,8 +4339,10 @@ PRE(sys_poll)
    for (i = 0; i < ARG2; i++) {
       PRE_MEM_READ( "poll(ufds.fd)",
                     (Addr)(&ufds[i].fd), sizeof(ufds[i].fd) );
-      PRE_MEM_READ( "poll(ufds.events)",
-                    (Addr)(&ufds[i].events), sizeof(ufds[i].events) );
+      if (ML_(safe_to_deref)(&ufds[i].fd, sizeof(ufds[i].fd)) && ufds[i].fd >= 0) {
+         PRE_MEM_READ( "poll(ufds.events)",
+                       (Addr)(&ufds[i].events), sizeof(ufds[i].events) );
+      }
       PRE_MEM_WRITE( "poll(ufds.revents)",
                      (Addr)(&ufds[i].revents), sizeof(ufds[i].revents) );
    }

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index f8621f8f0d1ca41ca4001a2e9b61a56ed07d50e5..20c68c877c49002a07b41cb7e63702872c629f4a 100644 (file)
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -1984,8 +1984,10 @@ static void ppoll_pre_helper ( ThreadId tid, SyscallArgLayout* layout,
    for (i = 0; i < ARG2; i++) {
       PRE_MEM_READ( "ppoll(ufds.fd)",
                     (Addr)(&ufds[i].fd), sizeof(ufds[i].fd) );
-      PRE_MEM_READ( "ppoll(ufds.events)",
-                    (Addr)(&ufds[i].events), sizeof(ufds[i].events) );
+      if (ufds[i].fd >= 0) {
+         PRE_MEM_READ( "ppoll(ufds.events)",
+                       (Addr)(&ufds[i].events), sizeof(ufds[i].events) );
+      }
       PRE_MEM_WRITE( "ppoll(ufds.revents)",
                      (Addr)(&ufds[i].revents), sizeof(ufds[i].revents) );
    }

diff --git a/coregrind/m_syswrap/syswrap-solaris.c b/coregrind/m_syswrap/syswrap-solaris.c
index 8a2a140f95f69b5c18b0128626279e36ef83ae81..ed3cb4a55180a3555754d87657eebfe1157ce0a0 100644 (file)
--- a/coregrind/m_syswrap/syswrap-solaris.c
+++ b/coregrind/m_syswrap/syswrap-solaris.c
@@ -7831,8 +7831,9 @@ PRE(sys_pollsys)
    for (i = 0; i < ARG2; i++) {
       vki_pollfd_t *u = &ufds[i];
       PRE_FIELD_READ("poll(ufds.fd)", u->fd);
-      /* XXX Check if it's valid? */
-      PRE_FIELD_READ("poll(ufds.events)", u->events);
+      if (ML_(safe_to_deref)(&ufds[i].fd, sizeof(ufds[i].fd)) && ufds[i].fd >= 0) {
+         PRE_FIELD_READ("poll(ufds.events)", u->events);
+      }
       PRE_FIELD_WRITE("poll(ufds.revents)", u->revents);
    }
 

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 5a17fd35d49a73a269ebc5ee2e72b0fbdb707652..307f47bd8e39a165852d058ac8314b7ee595798e 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -118,6 +118,7 @@ EXTRA_DIST = \
        bug340392.stderr.exp bug340392.vgtest \
        bug464969_d_demangle.stderr.exp bug464969_d_demangle.vgtest \
                bug464969_d_demangle.stdout.exp \
+       bug472219.stderr.exp bug472219.vgtest \
        calloc-overflow.stderr.exp calloc-overflow.vgtest\
        cdebug_zlib.stderr.exp cdebug_zlib.vgtest \
        cdebug_zlib_gnu.stderr.exp cdebug_zlib_gnu.vgtest \
@@ -415,6 +416,7 @@ check_PROGRAMS = \
        bug287260 \
        bug340392 \
        bug464969_d_demangle \
+       bug472219 \
        calloc-overflow \
        client-msg \
        clientperm \
@@ -566,6 +568,7 @@ leak_cpp_interior_SOURCES   = leak_cpp_interior.cpp
 accounting_CFLAGS      = $(AM_CFLAGS) @FLAG_W_NO_ALLOC_SIZE_LARGER_THAN@
 badfree_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_FREE_NONHEAP_OBJECT@
 bug155125_CFLAGS       = $(AM_CFLAGS) -Wno-unused-result @FLAG_W_NO_ALLOC_SIZE_LARGER_THAN@
+bug472219_CFLAGS       = $(AM_CFLAGS) @FLAG_W_NO_UNINITIALIZED@
 mallinfo_CFLAGS                = $(AM_CFLAGS) -Wno-deprecated-declarations
 malloc3_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_ALLOC_SIZE_LARGER_THAN@
 sbfragment_CFLAGS      = $(AM_CFLAGS) -Wno-deprecated-declarations
@@ -663,6 +666,7 @@ reach_thread_register_LDADD = -lpthread
 
 realloc_size_zero_CFLAGS       = $(AM_CFLAGS) @FLAG_W_NO_INCOMPATIBLE_POINTER_TYPES_DISCARDS_QUALIFIERS@
 realloc_size_zero_mismatch_SOURCES = realloc_size_zero_mismatch.cpp
+realloc_size_zero_mismatch_CXXFLAGS = $(AM_CXXFLAGS) @FLAG_W_NO_MISMATCHED_NEW_DELETE@
 
 resvn_stack_CFLAGS      = $(AM_CFLAGS) @FLAG_W_NO_UNINITIALIZED@
 

diff --git a/memcheck/tests/bug472219.c b/memcheck/tests/bug472219.c
new file mode 100644 (file)
index 0000000..88567ca
--- /dev/null
+++ b/memcheck/tests/bug472219.c
@@ -0,0 +1,16 @@
+#include <poll.h>
+#include <stdlib.h>
+#include "../../config.h"
+
+int main()
+{
+   int uninit;
+   struct pollfd fds[] = {{-1, uninit, 0}, {2, POLLIN, 0}};
+
+   poll(fds, 2, 100);
+
+#if defined(HAVE_PPOLL)
+   struct timespec timeout = {0, 1e8};
+   ppoll(fds, 2, &timeout, NULL);
+#endif
+}

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

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

diff --git a/memcheck/tests/freebsd/scalar.c b/memcheck/tests/freebsd/scalar.c
index c6a7ff2d5c1c91944bd1a9c6770578f5c58ad988..6c8d81aa6e14a2095ed4cab622c5ce2daf7d419c 100644 (file)
--- a/memcheck/tests/freebsd/scalar.c
+++ b/memcheck/tests/freebsd/scalar.c
@@ -781,9 +781,15 @@ int main(void)
    /* netbsd newreboot            208 */
 
    /* SYS_poll                    209 */
-   GO(SYS_poll, "3s 3m");
+   GO(SYS_poll, "2s 2m");
    SY(SYS_poll, x0, x0+1, x0); FAIL;
 
+   {
+      struct pollfd fds = { x0, x0, x0 };
+      GO(SYS_poll, "0s 2m");
+      SY(SYS_poll, &fds, 1, 1); SUCC;
+   }
+
    /* SYS_freebsd7___semctl       220 */
    GO(SYS_freebsd7___semctl, "(IPC_INFO) 4s 1m");
    SY(SYS_freebsd7___semctl, x0, x0, x0+IPC_INFO, x0+1); FAIL;
@@ -1948,8 +1954,8 @@ int main(void)
    {
        struct pollfd arg1;
        arg1.fd = arg1.events = arg1.revents = x0;
-        GO(SYS_ppoll, "2s 2+2m");
-        SY(SYS_ppoll, &arg1, 1, x0+1, x0+1); FAIL;
+       GO(SYS_ppoll, "2s 2+2m");
+       SY(SYS_ppoll, &arg1, 1, x0+1, x0+1); FAIL;
    }
 
    /* SYS_futimens                546 */

diff --git a/memcheck/tests/freebsd/scalar.stderr.exp b/memcheck/tests/freebsd/scalar.stderr.exp
index 2595bd38c530b73c9284ab1cc459f56f46b4c4aa..5a4f3230f13b0c19d5a82fa13f72d6dcd06cbebf 100644 (file)
--- a/memcheck/tests/freebsd/scalar.stderr.exp
+++ b/memcheck/tests/freebsd/scalar.stderr.exp
@@ -1529,7 +1529,7 @@ Syscall param getpgid(pid) contains uninitialised byte(s)
    ...
 
 ---------------------------------------------------------
-209:                SYS_poll 3s 3m
+209:                SYS_poll 2s 2m
 ---------------------------------------------------------
 Syscall param poll(ufds) contains uninitialised byte(s)
    ...
@@ -1544,13 +1544,20 @@ Syscall param poll(ufds.fd) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.events) points to unaddressable byte(s)
+Syscall param poll(ufds.revents) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.revents) points to unaddressable byte(s)
+---------------------------------------------------------
+209:                SYS_poll 0s 2m
+---------------------------------------------------------
+Syscall param poll(ufds.fd) points to uninitialised byte(s)
    ...
- Address 0x........ is not stack'd, malloc'd or (recently) free'd
+ Address 0x........ is on thread 1's stack
+
+Syscall param poll(ufds.events) points to uninitialised byte(s)
+   ...
+ Address 0x........ is on thread 1's stack
 
 ---------------------------------------------------------
 220:   SYS_freebsd7___semctl (IPC_INFO) 4s 1m
@@ -4968,6 +4975,14 @@ Syscall param ppoll(timeout) contains uninitialised byte(s)
 Syscall param ppoll(newsigmask) contains uninitialised byte(s)
    ...
 
+Syscall param ppoll(fds.fd) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
+Syscall param ppoll(fds.revents) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 Syscall param ppoll(timeout) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd

diff --git a/memcheck/tests/freebsd/scalar.stderr.exp-x86 b/memcheck/tests/freebsd/scalar.stderr.exp-x86
index e995fc28d605d6d3242ceee90d91688d84a44067..a45d0601c38e90313dc911d252b2944f2e8f8013 100644 (file)
--- a/memcheck/tests/freebsd/scalar.stderr.exp-x86
+++ b/memcheck/tests/freebsd/scalar.stderr.exp-x86
@@ -1529,7 +1529,7 @@ Syscall param getpgid(pid) contains uninitialised byte(s)
    ...
 
 ---------------------------------------------------------
-209:                SYS_poll 3s 3m
+209:                SYS_poll 2s 2m
 ---------------------------------------------------------
 Syscall param poll(ufds) contains uninitialised byte(s)
    ...
@@ -1544,13 +1544,20 @@ Syscall param poll(ufds.fd) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.events) points to unaddressable byte(s)
+Syscall param poll(ufds.revents) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.revents) points to unaddressable byte(s)
+---------------------------------------------------------
+209:                SYS_poll 0s 2m
+---------------------------------------------------------
+Syscall param poll(ufds.fd) points to uninitialised byte(s)
    ...
- Address 0x........ is not stack'd, malloc'd or (recently) free'd
+ Address 0x........ is on thread 1's stack
+
+Syscall param poll(ufds.events) points to uninitialised byte(s)
+   ...
+ Address 0x........ is on thread 1's stack
 
 ---------------------------------------------------------
 220:   SYS_freebsd7___semctl (IPC_INFO) 4s 1m
@@ -5023,6 +5030,14 @@ Syscall param ppoll(timeout) contains uninitialised byte(s)
 Syscall param ppoll(newsigmask) contains uninitialised byte(s)
    ...
 
+Syscall param ppoll(fds.fd) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
+Syscall param ppoll(fds.revents) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 Syscall param ppoll(timeout) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd

diff --git a/memcheck/tests/solaris/scalar.stderr.exp b/memcheck/tests/solaris/scalar.stderr.exp
index 1a04979d199b5df1c239ccce8da676bfe5c74640..a1b5d97d7ad4137a2fdb64c9db4174242f899743 100644 (file)
--- a/memcheck/tests/solaris/scalar.stderr.exp
+++ b/memcheck/tests/solaris/scalar.stderr.exp
@@ -3244,10 +3244,6 @@ Syscall param poll(ufds.fd) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.events) points to unaddressable byte(s)
-   ...
- Address 0x........ is not stack'd, malloc'd or (recently) free'd
-
 Syscall param poll(ufds.revents) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd

diff --git a/memcheck/tests/x86-linux/scalar.stderr.exp b/memcheck/tests/x86-linux/scalar.stderr.exp
index b9202a8c2f9cacf8c8812671584c3b2aa599d6f5..6b8c7677f55501894137f411d44df92b40cd1715 100644 (file)
--- a/memcheck/tests/x86-linux/scalar.stderr.exp
+++ b/memcheck/tests/x86-linux/scalar.stderr.exp
@@ -2122,11 +2122,6 @@ Syscall param poll(ufds.fd) points to unaddressable byte(s)
    by 0x........: main (scalar.c:761)
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
-Syscall param poll(ufds.events) points to unaddressable byte(s)
-   ...
-   by 0x........: main (scalar.c:761)
- Address 0x........ is not stack'd, malloc'd or (recently) free'd
-
 Syscall param poll(ufds.revents) points to unaddressable byte(s)
    ...
    by 0x........: main (scalar.c:761)