Bug 418840 - SIG_IGN doesn't clear pending signal if SIG_IGN is already the handler
Summary: SIG_IGN doesn't clear pending signal if SIG_IGN is already the handler
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: 3.15 SVN
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-14 05:50 UTC by Pavel Roskin
Modified: 2020-03-14 18:11 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Demo program (1.18 KB, text/x-csrc)
2020-03-14 05:50 UTC, Pavel Roskin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Roskin 2020-03-14 05:50:00 UTC
Created attachment 126773 [details]
Demo program

SUMMARY

There is a difference between native Linux and Valgrind w.r.t. signal handling. 
If a signal is blocked and pending, its handler is SIG_IGN and another SIG_IGN handler is installed, Linux clears the signal but Valgrind doesn't.

I don't know if there is a specified behavior, but I believe the value of the old handler should play no role. The behavior of Valgrind depends on the old handler. If the old handler is not SIG_IGN, the pending signal is cleared.

I suspect the logic in the Valgrind code skips some actions if the signal handler is unchanged, but it skips the signal clearing incorrectly.

STEPS TO REPRODUCE
1. Block a signal
2. Install the SIG_IGN handler
3. Raise that signal
4. Install the SIG_IGN handler again
5. Check if the signal is pending

See the attached program that demonstrated the issue.

OBSERVED RESULT
The signal is pending

The last status line of the demo program is
Signal: pending, blocked, ignored

EXPECTED RESULT
The signal is not pending

The last status line of the demo program is
Signal: not pending, blocked, ignored

SOFTWARE/OS VERSIONS
Valgrind 3.15.0 (also today's git master branch)
Ubuntu 18.04
Linux roskinp-p7510 5.3.0-40-generic #32~18.04.1-Ubuntu SMP Mon Feb 3 14:05:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

ADDITIONAL INFORMATION

roskinp@roskinp-p7510:~/src/valgrind-signal$ ./signaltest
Signal: not pending, unblocked, handled
Blocking signal
Signal: not pending, blocked, handled
Ignoring signal
Signal: not pending, blocked, ignored
Raising signal
Signal: pending, blocked, ignored
Ignoring signal
Signal: not pending, blocked, ignored
Finished
roskinp@roskinp-p7510:~/src/valgrind-signal$ /home/roskinp/valgrind/bin/valgrind -v ./signaltest 
==7609== Memcheck, a memory error detector
==7609== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==7609== Using Valgrind-3.16.0.GIT-95df121886-20200313 and LibVEX; rerun with -h for copyright info
==7609== Command: ./signaltest
==7609== 
--7609-- Valgrind options:
--7609--    -v
--7609-- Contents of /proc/version:
--7609--   Linux version 5.3.0-40-generic (buildd@lcy01-amd64-024) (gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)) #32~18.04.1-Ubuntu SMP Mon Feb 3 14:05:59 UTC 2020
--7609-- 
--7609-- Arch and hwcaps: AMD64, LittleEndian, amd64-cx16-lzcnt-rdtscp-sse3-ssse3-avx-avx2-bmi-f16c-rdrand
--7609-- Page sizes: currently 4096, max supported 4096
--7609-- Valgrind library directory: /home/roskinp/valgrind/lib/valgrind
--7609-- Reading syms from /home/roskinp/src/valgrind-signal/signaltest
--7609-- Reading syms from /lib/x86_64-linux-gnu/ld-2.27.so
--7609--   Considering /lib/x86_64-linux-gnu/ld-2.27.so ..
--7609--   .. CRC mismatch (computed 1b7c895e wanted 2943108a)
--7609--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/ld-2.27.so ..
--7609--   .. CRC is valid
--7609-- Reading syms from /home/roskinp/valgrind/lib/valgrind/memcheck-amd64-linux
--7609--    object doesn't have a dynamic symbol table
--7609-- Scheduler: using generic scheduler lock implementation.
--7609-- Reading suppressions file: /home/roskinp/valgrind/lib/valgrind/default.supp
==7609== embedded gdbserver: reading from /tmp/vgdb-pipe-from-vgdb-to-7609-by-roskinp-on-???
==7609== embedded gdbserver: writing to   /tmp/vgdb-pipe-to-vgdb-from-7609-by-roskinp-on-???
==7609== embedded gdbserver: shared mem   /tmp/vgdb-pipe-shared-mem-vgdb-7609-by-roskinp-on-???
==7609== 
==7609== TO CONTROL THIS PROCESS USING vgdb (which you probably
==7609== don't want to do, unless you know exactly what you're doing,
==7609== or are doing some strange experiment):
==7609==   /home/roskinp/valgrind/lib/valgrind/../../bin/vgdb --pid=7609 ...command...
==7609== 
==7609== TO DEBUG THIS PROCESS USING GDB: start GDB like this
==7609==   /path/to/gdb ./signaltest
==7609== and then give GDB the following command
==7609==   target remote | /home/roskinp/valgrind/lib/valgrind/../../bin/vgdb --pid=7609
==7609== --pid is optional if only one valgrind process is running
==7609== 
--7609-- REDIR: 0x401f2f0 (ld-linux-x86-64.so.2:strlen) redirected to 0x58119398 (vgPlain_amd64_linux_REDIR_FOR_strlen)
--7609-- REDIR: 0x401f0d0 (ld-linux-x86-64.so.2:index) redirected to 0x581193b2 (vgPlain_amd64_linux_REDIR_FOR_index)
--7609-- Reading syms from /home/roskinp/valgrind/lib/valgrind/vgpreload_core-amd64-linux.so
--7609-- Reading syms from /home/roskinp/valgrind/lib/valgrind/vgpreload_memcheck-amd64-linux.so
==7609== WARNING: new redirection conflicts with existing -- ignoring it
--7609--     old: 0x0401f2f0 (strlen              ) R-> (0000.0) 0x58119398 vgPlain_amd64_linux_REDIR_FOR_strlen
--7609--     new: 0x0401f2f0 (strlen              ) R-> (2007.0) 0x04c33190 strlen
--7609-- REDIR: 0x401d360 (ld-linux-x86-64.so.2:strcmp) redirected to 0x4c34240 (strcmp)
--7609-- REDIR: 0x401f830 (ld-linux-x86-64.so.2:mempcpy) redirected to 0x4c37c40 (mempcpy)
--7609-- Reading syms from /lib/x86_64-linux-gnu/libc-2.27.so
--7609--   Considering /lib/x86_64-linux-gnu/libc-2.27.so ..
--7609--   .. CRC mismatch (computed b1c74187 wanted 042cc048)
--7609--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/libc-2.27.so ..
--7609--   .. CRC is valid
--7609-- REDIR: 0x4edbc70 (libc.so.6:memmove) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edad40 (libc.so.6:strncpy) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbf50 (libc.so.6:strcasecmp) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4eda790 (libc.so.6:strcat) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edad70 (libc.so.6:rindex) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edd7c0 (libc.so.6:rawmemchr) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4efa410 (libc.so.6:wmemchr) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbde0 (libc.so.6:mempcpy) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbc10 (libc.so.6:bcmp) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edad00 (libc.so.6:strncmp) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4eda800 (libc.so.6:strcmp) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbd40 (libc.so.6:memset) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4ef90f0 (libc.so.6:wcschr) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edaca0 (libc.so.6:strnlen) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4eda870 (libc.so.6:strcspn) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbfa0 (libc.so.6:strncasecmp) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4eda840 (libc.so.6:strcpy) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edc0e0 (libc.so.6:memcpy@@GLIBC_2.14) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4efb6c0 (libc.so.6:wcsnlen) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edada0 (libc.so.6:strpbrk) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4eda7c0 (libc.so.6:index) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edac70 (libc.so.6:strlen) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4ee56c0 (libc.so.6:memrchr) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbff0 (libc.so.6:strcasecmp_l) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbbe0 (libc.so.6:memchr) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4ef9eb0 (libc.so.6:wcslen) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edb050 (libc.so.6:strspn) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbf20 (libc.so.6:stpncpy) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edbef0 (libc.so.6:stpcpy) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edd7f0 (libc.so.6:strchrnul) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4edc040 (libc.so.6:strncasecmp_l) redirected to 0x4a2a740 (_vgnU_ifunc_wrapper)
--7609-- REDIR: 0x4fcb3c0 (libc.so.6:__strrchr_avx2) redirected to 0x4c32b20 (rindex)
--7609-- REDIR: 0x4fcb1d0 (libc.so.6:__strchrnul_avx2) redirected to 0x4c37770 (strchrnul)
--7609-- REDIR: 0x4ed4070 (libc.so.6:malloc) redirected to 0x4c2febe (malloc)
--7609-- REDIR: 0x4fcb590 (libc.so.6:__strlen_avx2) redirected to 0x4c330d0 (strlen)
--7609-- REDIR: 0x4fcbab0 (libc.so.6:__mempcpy_avx_unaligned_erms) redirected to 0x4c37880 (mempcpy)
Signal: not pending, unblocked, handled
Blocking signal
Signal: not pending, blocked, handled
Ignoring signal
Signal: not pending, blocked, ignored
Raising signal
Signal: pending, blocked, ignored
Ignoring signal
Signal: pending, blocked, ignored
Finished
--7609-- REDIR: 0x4ed4950 (libc.so.6:free) redirected to 0x4c30fb8 (free)
==7609== 
==7609== HEAP SUMMARY:
==7609==     in use at exit: 0 bytes in 0 blocks
==7609==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==7609== 
==7609== All heap blocks were freed -- no leaks are possible
==7609== 
==7609== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Comment 1 Philippe Waroquiers 2020-03-14 18:11:14 UTC
I think the bug originates from the lazy translation of the scss to skss,
in function static void handle_SCSS_change ( Bool force_update ).

When force_update is false (which is the case when handling a guest 
sigaction call), and the old setup is equal to the new setup of the
kernel signal state, then no kernel sigaction is executed.

So, when a signal is blocked+ignored then raised,
the signal is queued by the kernel till it is unblocked (and then
it will be ignored).

In this state, if a blocked signal is re-ignored, the kernel clears it.
But in the same circumstance, valgrind signal handling does not call sigaction:
valgrind does not know that there is a blocked ignored queued signal
in the kernel, it just sees that the signal handling is not changed,
and then it does not call the kernel to tell to re-ignore the signal.

Changing the call of handle_SCSS_change in sys_sigaction to always use
force_update solves the problem, but that means the lazy update of
the signal handling is removed.

It would be possible to detect this case in handle_SCSS_change:
if the current setup is blocked + ignored, then this function
could check if there is a signal pending, and then it should still
call the kernel to clear the blocked signal.
That means one more syscall to get the pending signals in case one or more
signals are blocked + ignored.