Skip to content

CWE-479 in CRT.c #1563

@BenBE

Description

@BenBE
Member

While compiling I noticed the following warning for CWE-479 from GCC 14:

depbase=`echo CRT.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc-14 -DHAVE_CONFIG_H -I.  -DNDEBUG  -std=c99 -pedantic -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600  -I/usr/include/libnl3 -Wall -Wcast-align -Wcast-qual -Wextra -Wfloat-equal -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wpointer-arith -Wshadow -Wstrict-prototypes -Wundef -Wunused -Wwrite-strings -Wnull-dereference -D_XOPEN_SOURCE_EXTENDED -DSYSCONFDIR="\"/usr/local/etc\"" -I"./linux" -fanalyzer -MT CRT.o -MD -MP -MF $depbase.Tpo -c -o CRT.o CRT.c &&\
mv -f $depbase.Tpo $depbase.Po
CRT.c: In function ‘CRT_handleSIGTERM’:
CRT.c:846:4: warning: call to ‘snprintf’ from within signal handler [CWE-479] [-Wanalyzer-unsafe-call-within-signal-handler]
  846 |    snprintf(err_buf, sizeof(err_buf),
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  847 |            "A signal %d (%s) was received, exiting without persisting settings to htoprc.\n",
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  848 |            sgn, signal_str);
      |            ~~~~~~~~~~~~~~~~
  ‘CRT_installSignalHandlers’: events 1-2
    |
    |  952 | static void CRT_installSignalHandlers(void) {
    |      |             ^~~~~~~~~~~~~~~~~~~~~~~~~
    |      |             |
    |      |             (1) entry to ‘CRT_installSignalHandlers’
    |......
    |  966 |    signal(SIGINT, CRT_handleSIGTERM);
    |      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |    |
    |      |    (2) registering ‘CRT_handleSIGTERM’ as signal handler
    |
  event 3
    |
    |cc1:
    | (3): later on, when the signal is delivered to the process
    |
    +--> ‘CRT_handleSIGTERM’: events 4-7
           |
           |  835 | static void CRT_handleSIGTERM(int sgn) {
           |      |             ^~~~~~~~~~~~~~~~~
           |      |             |
           |      |             (4) entry to ‘CRT_handleSIGTERM’
           |......
           |  838 |    if (!CRT_settings->changed)
           |      |       ~      
           |      |       |
           |      |       (5) following ‘false’ branch...
           |......
           |  841 |    const char* signal_str = strsignal(sgn);
           |      |                             ~~~~~~~~~~~~~~
           |      |                             |
           |      |                             (6) ...to here
           |......
           |  846 |    snprintf(err_buf, sizeof(err_buf),
           |      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |    |
           |      |    (7) call to ‘snprintf’ from within signal handler
           |  847 |            "A signal %d (%s) was received, exiting without persisting settings to htoprc.\n",
           |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |  848 |            sgn, signal_str);
           |      |            ~~~~~~~~~~~~~~~~
           |

Activity

cgzones

cgzones commented on Nov 15, 2024

@cgzones
Member

Out of curiosity I asked an AI:

Based on the search results, snprintf is not considered async-signal-safe in C. Here’s a breakdown of the reasons:

  • Valgrind bug: The snprintf function may not work correctly when called from a signal handler due to valgrind’s internal implementation. This is a specific issue with valgrind, not a fundamental problem with snprintf.
  • ABI requirements: The x86-64 ABI requires floating-point arguments to be passed in XMM registers, which must be 16-byte aligned. Signal handlers may not provide this alignment, making snprintf unreliable.
  • Non-async-signal-safe functions: snprintf is not listed as an async-signal-safe function in the POSIX standard or other reliable sources. This implies that it may not be safe to use in signal handlers.
  • Initialization of locale machinery: snprintf may need to initialize locale machinery, which is not async-signal-safe.

To work around these issues, consider the following alternatives:

  • Use a limited, async-signal-safe version of snprintf: The safe_snprintf function provided by idning/safe_snprintf is a simple, limited version of snprintf that is designed to be async-signal-safe. It supports a limited set of formats and does not initialize locale machinery.
  • Set a flag and handle it in the main program: Instead of calling snprintf directly from a signal handler, set a flag inside the signal handler and have the main program check and handle the flag accordingly.
  • Use write instead of printf: On POSIX-conforming systems, you can use write(STDOUT_FILENO, ...) instead of printf to output data from a signal handler. This approach may require additional formatting effort.

In summary, while snprintf is not inherently async-signal-safe, there are workarounds and alternatives available to ensure safe and reliable signal handling in C programs.

Explorer09

Explorer09 commented on Nov 25, 2024

@Explorer09
Contributor

Since the CRT_handleSIGTERM function simply exits after printing the diagnostic message, I wonder if the problem can be fixed by simply blocking other signals before calling snprintf(3)?

I mean, will the warning go away if we fix that way?

BenBE

BenBE commented on Nov 25, 2024

@BenBE
MemberAuthor

This wont do …

linked a pull request that will close this issue on Dec 16, 2024
Explorer09

Explorer09 commented on Mar 11, 2025

@Explorer09
Contributor

Related to this bug. The signal_safe_fprintf() function in Settings.c is not quite "signal safe" as the function says, and should be fixed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @BenBE@Explorer09@cgzones

      Issue actions

        CWE-479 in CRT.c · Issue #1563 · htop-dev/htop