| Summary: | Syscall param ppoll(ufds.events) points to uninitialised byte(s) | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | fanquake |
| Component: | general | Assignee: | Paul Floyd <pjfloyd> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | alice, kde.sigfw, pjfloyd |
| Priority: | NOR | ||
| Version: | 3.21.0 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
initial patch
with missing target in makefile patch for poll negative fd patch for poll and negative fds Solaris scalar expected Linux scalar |
||
I'm a bit confused. How do you get an error in ppoll when all you call is getaddrinfo? According to https://elixir.bootlin.com/musl/v1.2.4/A/ident/ppoll the only call to ppoll is https://elixir.bootlin.com/musl/v1.2.4/source/compat/time32/ppoll_time32.c#L8 (maybe the search doesn't work so well) According to Valgrind, ppoll gets passed an int representing the number of fds and a corresponding array of pollfd, and it's saying that one of the 'events' fields is uninitialized. Since it looks like this is internal to musl it will be fairly difficult to debug without a debug build of musl. On libera.chat #musl I asked about this and on some architectures poll calls SYS_ppoll
Debug trace:
$ valgrind -s --track-origins=yes --gen-suppressions=all ./a.out
==7940== Memcheck, a memory error detector
==7940== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==7940== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info
==7940== Command: ./a.out
==7940==
==7940== Warning: set address range perms: large range [0x48b1000, 0x14d0e000) (defined)
==7940== Warning: set address range perms: large range [0x48c4000, 0x14d0e000) (defined)
==7940== Warning: set address range perms: large range [0x48c6000, 0x14d0e000) (defined)
==7940== Syscall param poll(ufds.events) points to uninitialised byte(s)
==7940== at 0x4054F0A: ??? (syscall_cp.s:29)
==7940== by 0x40520ED: __syscall_cp_c (pthread_cancel.c:33)
==7940== by 0x4045BF9: poll (poll.c:9)
==7940== by 0x403B012: __res_msend_rc (res_msend.c:197)
==7940== by 0x40392FC: name_from_dns (lookup_name.c:171)
==7940== by 0x4039A3C: name_from_dns_search (lookup_name.c:231)
==7940== by 0x4039A3C: __lookup_name (lookup_name.c:334)
==7940== by 0x4036EA8: getaddrinfo (getaddrinfo.c:94)
==7940== by 0x1091F1: main (in /tmp/mytemp.apEOmk/a.out)
==7940== Address 0x1ffeffe834 is on thread 1's stack
==7940== in frame #3, created by __res_msend_rc (res_msend.c:82)
==7940== Uninitialised value was created by a stack allocation
==7940== at 0x403A9EA: __res_msend_rc (res_msend.c:82)
==7940==
{
<insert_a_suppression_name_here>
Memcheck:Param
poll(ufds.events)
obj:/lib/ld-musl-x86_64.so.1
fun:__syscall_cp_c
fun:poll
fun:__res_msend_rc
fun:name_from_dns
fun:name_from_dns_search
fun:__lookup_name
fun:getaddrinfo
fun:main
}
==7940==
==7940== HEAP SUMMARY:
==7940== in use at exit: 0 bytes in 0 blocks
==7940== total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==7940==
==7940== All heap blocks were freed -- no leaks are possible
==7940==
==7940== ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)
==7940==
==7940== 4 errors in context 1 of 1:
==7940== Syscall param poll(ufds.events) points to uninitialised byte(s)
==7940== at 0x4054F0A: ??? (syscall_cp.s:29)
==7940== by 0x40520ED: __syscall_cp_c (pthread_cancel.c:33)
==7940== by 0x4045BF9: poll (poll.c:9)
==7940== by 0x403B012: __res_msend_rc (res_msend.c:197)
==7940== by 0x40392FC: name_from_dns (lookup_name.c:171)
==7940== by 0x4039A3C: name_from_dns_search (lookup_name.c:231)
==7940== by 0x4039A3C: __lookup_name (lookup_name.c:334)
==7940== by 0x4036EA8: getaddrinfo (getaddrinfo.c:94)
==7940== by 0x1091F1: main (in /tmp/mytemp.apEOmk/a.out)
==7940== Address 0x1ffeffe834 is on thread 1's stack
==7940== in frame #3, created by __res_msend_rc (res_msend.c:82)
==7940== Uninitialised value was created by a stack allocation
==7940== at 0x403A9EA: __res_msend_rc (res_msend.c:82)
==7940==
{
<insert_a_suppression_name_here>
Memcheck:Param
poll(ufds.events)
obj:/lib/ld-musl-x86_64.so.1
fun:__syscall_cp_c
fun:poll
fun:__res_msend_rc
fun:name_from_dns
fun:name_from_dns_search
fun:__lookup_name
fun:getaddrinfo
fun:main
}
==7940== ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)
The source code in question is probably here
https://elixir.bootlin.com/musl/v1.2.4/source/src/network/res_msend.c#L197
for unrelated context to alpine, `apk add musl-dbg` will generate a more useful trace with the function calls inside libc when running valgrind (or using a debugger, ..), just for future reference This does look like a Valgrind bug:
The manpage says
The field fd contains a file descriptor for an open file. If this field is negative, then the corresponding events field is ignored and the revents field returns zero. (This provides an easy way of ignoring a file descriptor for a single poll() call: simply negate the fd field.)
But we don't check for that:
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) );
PRE_MEM_WRITE( "ppoll(ufds.revents)",
(Addr)(&ufds[i].revents), sizeof(ufds[i].revents) );
}
I think that should be
for (i = 0; i < ARG2; i++) {
PRE_MEM_READ( "ppoll(ufds.fd)",
(Addr)(&ufds[i].fd), sizeof(ufds[i].fd) );
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) );
}
The behaviour for -ve fd is also specified by POSIX (Open Group Base Spec 7 2018 at least). (In reply to Paul Floyd from comment #4) > This does look like a Valgrind bug: > > The manpage says > > The field fd contains a file descriptor for an open file. If this field is > negative, then the corresponding events field is ignored and the revents > field returns zero. (This provides an easy way of ignoring a file descriptor > for a single poll() call: simply negate the fd field.) > > But we don't check for that: > > 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) ); > PRE_MEM_WRITE( "ppoll(ufds.revents)", > (Addr)(&ufds[i].revents), sizeof(ufds[i].revents) ); > } > > I think that should be > > for (i = 0; i < ARG2; i++) { > PRE_MEM_READ( "ppoll(ufds.fd)", > (Addr)(&ufds[i].fd), sizeof(ufds[i].fd) ); > 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) ); > } Compiling master + your patch fixes the issue for me, using the test case provided. Solaris has this comment
/* XXX Check if it's valid? */
which was right, it needed checking.
Created attachment 160471 [details]
initial patch
Not yet updated Solaris and Linux scalar expecteds.
Created attachment 160474 [details]
with missing target in makefile
Still without macOS Solaris and Linux updates to expecteds
Created attachment 160475 [details]
patch for poll negative fd
Remove uninitialized warning
Created attachment 160476 [details]
patch for poll and negative fds
This time compiles on macOS
Created attachment 160478 [details]
Solaris scalar expected
Created attachment 160496 [details]
Linux scalar
commit 6ce0979884a8f246c80a098333ceef1a7b7f694d (HEAD -> master, origin/master, origin/HEAD) Author: Paul Floyd <pjfloyd@wanadoo.fr> Date: Mon Jul 24 22:06:00 2023 +0200 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. |
aarch64 Alpine Linux (musl libc 1.2.4) gcc (Alpine 12.2.1_git20220924-r10) 12.2.1 20220924 valgrind-3.21.0 Using the following code, compiled with "g++ test.cpp": ```cpp #include <sys/types.h> #include <sys/socket.h> #include <netdb.h> int main(){ addrinfo ai_hint{}; ai_hint.ai_socktype = SOCK_STREAM; ai_hint.ai_protocol = IPPROTO_TCP; ai_hint.ai_family = AF_UNSPEC; ai_hint.ai_flags = true? AI_ADDRCONFIG : AI_NUMERICHOST; addrinfo* ai_res{nullptr}; return getaddrinfo("foo.bar", nullptr, &ai_hint, &ai_res); } ``` valgrind -s --track-origins=yes --gen-suppressions=all ./a.out ==79== Memcheck, a memory error detector ==79== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==79== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info ==79== Command: ./a.out ==79== ==79== Syscall param ppoll(ufds.events) points to uninitialised byte(s) ==79== at 0x405DCCC: ??? (in /lib/ld-musl-aarch64.so.1) ==79== Address 0x1ffefff854 is on thread 1's stack ==79== Uninitialised value was created by a stack allocation ==79== at 0x404498C: ??? (in /lib/ld-musl-aarch64.so.1) ==79== { <insert_a_suppression_name_here> Memcheck:Param ppoll(ufds.events) obj:/lib/ld-musl-aarch64.so.1 } ==79== ==79== HEAP SUMMARY: ==79== in use at exit: 0 bytes in 0 blocks ==79== total heap usage: 0 allocs, 0 frees, 0 bytes allocated ==79== ==79== All heap blocks were freed -- no leaks are possible ==79== ==79== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) ==79== ==79== 1 errors in context 1 of 1: ==79== Syscall param ppoll(ufds.events) points to uninitialised byte(s) ==79== at 0x405DCCC: ??? (in /lib/ld-musl-aarch64.so.1) ==79== Address 0x1ffefff854 is on thread 1's stack ==79== Uninitialised value was created by a stack allocation ==79== at 0x404498C: ??? (in /lib/ld-musl-aarch64.so.1) ==79== { <insert_a_suppression_name_here> Memcheck:Param ppoll(ufds.events) obj:/lib/ld-musl-aarch64.so.1 } ==79== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)