Bug 364413 - pselect sycallwrapper mishandles NULL sigmask
Summary: pselect sycallwrapper mishandles NULL sigmask
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.11 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL: https://bugzilla.redhat.com/show_bug....
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-17 09:09 UTC by Mark Wielaard
Modified: 2016-07-11 21:11 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Proposed testcase and fix (4.86 KB, patch)
2016-06-17 09:25 UTC, Mark Wielaard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2016-06-17 09:09:16 UTC
This is a regression caused by valgrind svn r15823 which fixed Bug 359871 "sanitize signal mask in ppoll and pselect syscalls".

The issue is caused by this C library/kernel ABI differences according to http://man7.org/linux/man-pages/man2/pselect6.2.html

       The Linux pselect6() system call modifies its timeout argument.
       However, the glibc wrapper function hides this behavior by using a
       local variable for the timeout argument that is passed to the system
       call.  Thus, the glibc pselect() function does not modify its timeout
       argument; this is the behavior required by POSIX.1-2001.

       The final argument of the pselect6() system call is not a sigset_t *
       pointer, but is instead a structure of the form:

           struct {
               const sigset_t *ss;     /* Pointer to signal set */
               size_t          ss_len; /* Size (in bytes) of object pointed
                                          to by 'ss' */
           };

       This allows the system call to obtain both a pointer to the signal
       set and its size, while allowing for the fact that most architectures
       support a maximum of 6 arguments to a system call.

What we are seeing is glibc modifying the timeout argument (NULL) and passing it as a struct { NULL, 8 } (where 8 is the correct ss_len if ss wouldn't be NULL).

valgrind doesn't check whether ss is NULL before calling PRE_MEM_READ on it and so generates a bogus warning.

Reproducible: Always
Comment 1 Mark Wielaard 2016-06-17 09:25:49 UTC
Created attachment 99546 [details]
Proposed testcase and fix

When the last argument ss == NULL don't check whether it is valid and don't try to copy the sigmask (which would crash because it is NULL), just pass through the NULL sigmask to the kernel. Testcase provided by Paul Eggert in the original bug report https://bugzilla.redhat.com/show_bug.cgi?id=1344082
Comment 2 Mark Wielaard 2016-06-21 20:10:38 UTC
valgrind svn 15893
Comment 3 Mark Wielaard 2016-06-21 21:07:49 UTC
I made a typo which caused:

...checking makefile consistency
none/tests/Makefile.am:1: error: pselect_sigmask_null.vgtest is missing in EXTRA_DIST
none/tests/Makefile.am:1: error: pselect_signask_null.vgtest is in EXTRA_DIST but doesn't exist

Fixed with valgrind svn r15894
Bug 364413 followup - fix signask -> sigmask typo in EXTRA_DIST
Comment 4 Ivo Raisr 2016-07-11 21:11:15 UTC
Test executable ignored in SVN r15904.