From: Paul Floyd Date: Mon, 24 Jul 2023 20:06:00 +0000 (+0200) Subject: Bug 472219 - Syscall param ppoll(ufds.events) points to uninitialised byte(s) X-Git-Tag: VALGRIND_3_22_0~133 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6ce0979884a8f246c80a098333ceef1a7b7f694d;p=thirdparty%2Fvalgrind.git 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 9e16ac126d..6538eb718b 100644 --- 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 783612fbb9..867d2f0f43 100644 --- 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 6b9f3d2109..9af37cfb83 100644 --- 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 efdae60e10..ed9d14685f 100644 --- 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 f8621f8f0d..20c68c877c 100644 --- 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 8a2a140f95..ed3cb4a551 100644 --- 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 5a17fd35d4..307f47bd8e 100644 --- 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 index 0000000000..88567caa2c --- /dev/null +++ b/memcheck/tests/bug472219.c @@ -0,0 +1,16 @@ +#include +#include +#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 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/bug472219.vgtest b/memcheck/tests/bug472219.vgtest new file mode 100644 index 0000000000..8cd48c785d --- /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 c6a7ff2d5c..6c8d81aa6e 100644 --- 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 2595bd38c5..5a4f3230f1 100644 --- 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 e995fc28d6..a45d0601c3 100644 --- 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 1a04979d19..a1b5d97d7a 100644 --- 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 b9202a8c2f..6b8c7677f5 100644 --- 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)