@@ -76,6 +76,7 @@ analyzer_OBJS = \
analyzer/sm-malloc.o \
analyzer/sm-pattern-test.o \
analyzer/sm-sensitive.o \
+ analyzer/sm-signal.o \
analyzer/sm-taint.o \
analyzer/state-purge.o \
analyzer/supergraph.o \
@@ -70,6 +70,10 @@ Wanalyzer-possible-null-dereference
Common Var(warn_analyzer_possible_null_dereference) Init(1) Warning
Warn about code paths in which a possibly-NULL pointer is dereferenced.
+Wanalyzer-unsafe-call-within-signal-handler
+Common Var(warn_analyzer_unsafe_call_within_signal_handler) Init(1) Warning
+Warn about code paths in which an async-signal-unsafe function is called from a signal handler.
+
Wanalyzer-null-argument
Common Var(warn_analyzer_null_argument) Init(1) Warning
Warn about code paths in which NULL is passed to a must-not-be-NULL function argument.
new file mode 100644
@@ -0,0 +1,304 @@
+/* An experimental state machine, for tracking bad calls from within
+ signal handlers.
+
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+#include "config.h"
+#include "gcc-plugin.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "gimple.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "analyzer/analyzer.h"
+#include "analyzer/checker-path.h"
+#include "analyzer/exploded-graph.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/sm.h"
+
+namespace {
+
+/* An experimental state machine, for tracking calls to async-signal-unsafe
+ functions from within signal handlers. */
+
+class signal_state_machine : public state_machine
+{
+public:
+ signal_state_machine (logger *logger);
+
+ bool inherited_state_p () const FINAL OVERRIDE { return false; }
+
+ bool on_stmt (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt) const FINAL OVERRIDE;
+
+ void on_condition (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt,
+ tree lhs,
+ enum tree_code op,
+ tree rhs) const FINAL OVERRIDE;
+
+ bool can_purge_p (state_t s) const FINAL OVERRIDE;
+
+ /* These states are "global", rather than per-expression. */
+
+ /* Start state. */
+ state_t m_start;
+
+ /* State for when we're in a signal handler. */
+ state_t m_in_signal_handler;
+
+ /* Stop state. */
+ state_t m_stop;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+/* Concrete subclass for describing call to an async-signal-unsafe function
+ from a signal handler. */
+
+class signal_unsafe_call
+ : public pending_diagnostic_subclass<signal_unsafe_call>
+{
+public:
+ signal_unsafe_call (const signal_state_machine &sm, const gcall *unsafe_call,
+ tree unsafe_fndecl)
+ : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl)
+ {
+ gcc_assert (m_unsafe_fndecl);
+ }
+
+ const char *get_kind () const FINAL OVERRIDE { return "signal_unsafe_call"; }
+
+ bool operator== (const signal_unsafe_call &other) const
+ {
+ return m_unsafe_call == other.m_unsafe_call;
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ diagnostic_metadata m;
+ /* CWE-479: Signal Handler Use of a Non-reentrant Function. */
+ m.add_cwe (479);
+ return warning_at (rich_loc, m,
+ OPT_Wanalyzer_unsafe_call_within_signal_handler,
+ "call to %qD from within signal handler",
+ m_unsafe_fndecl);
+ }
+
+ label_text describe_state_change (const evdesc::state_change &change)
+ FINAL OVERRIDE
+ {
+ if (change.is_global_p ()
+ && change.m_new_state == m_sm.m_in_signal_handler)
+ {
+ function *handler
+ = change.m_event.m_dst_state.m_region_model->get_current_function ();
+ return change.formatted_print ("registering %qD as signal handler",
+ handler->decl);
+ }
+ return label_text ();
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("call to %qD from within signal handler",
+ m_unsafe_fndecl);
+ }
+
+private:
+ const signal_state_machine &m_sm;
+ const gcall *m_unsafe_call;
+ tree m_unsafe_fndecl;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+/* signal_state_machine's ctor. */
+
+signal_state_machine::signal_state_machine (logger *logger)
+: state_machine ("signal", logger)
+{
+ m_start = add_state ("start");
+ m_in_signal_handler = add_state ("in_signal_handler");
+ m_stop = add_state ("stop");
+}
+
+/* Update MODEL for edges that simulate HANDLER_FUN being called as
+ an signal-handler in response to a signal. */
+
+static void
+update_model_for_signal_handler (region_model *model,
+ function *handler_fun)
+{
+ /* Purge all state within MODEL. */
+ *model = region_model ();
+ model->push_frame (handler_fun, NULL, NULL);
+}
+
+/* Custom exploded_edge info: entry into a signal-handler. */
+
+class signal_delivery_edge_info_t : public exploded_edge::custom_info_t
+{
+public:
+ void print (pretty_printer *pp) FINAL OVERRIDE
+ {
+ pp_string (pp, "signal delivered");
+ }
+
+ void update_model (region_model *model,
+ const exploded_edge &eedge) FINAL OVERRIDE
+ {
+ update_model_for_signal_handler (model, eedge.m_dest->get_function ());
+ }
+
+ void add_events_to_path (checker_path *emission_path,
+ const exploded_edge &eedge ATTRIBUTE_UNUSED)
+ {
+ emission_path->add_event
+ (new custom_event (UNKNOWN_LOCATION, NULL_TREE, 0,
+ "later on,"
+ " when the signal is delivered to the process"));
+ }
+};
+
+/* Concrete subclass of custom_transition for modeling registration of a
+ signal handler and the signal handler later being called. */
+
+class register_signal_handler : public custom_transition
+{
+public:
+ register_signal_handler (const signal_state_machine &sm,
+ tree fndecl)
+ : m_sm (sm), m_fndecl (fndecl) {}
+
+ /* Model a signal-handler FNDECL being called at some later point
+ by injecting an edge to a new function-entry node with an empty
+ callstring, setting the 'in-signal-handler' global state
+ on the node. */
+ void impl_transition (exploded_graph *eg,
+ exploded_node *src_enode,
+ int sm_idx) FINAL OVERRIDE
+ {
+ function *handler_fun = DECL_STRUCT_FUNCTION (m_fndecl);
+ if (!handler_fun)
+ return;
+ program_point entering_handler
+ = program_point::from_function_entry (eg->get_supergraph (),
+ handler_fun);
+
+ program_state state_entering_handler (eg->get_ext_state ());
+ update_model_for_signal_handler (state_entering_handler.m_region_model,
+ handler_fun);
+ state_entering_handler.m_checker_states[sm_idx]->set_global_state
+ (m_sm.m_in_signal_handler);
+
+ exploded_node *dst_enode = eg->get_or_create_node (entering_handler,
+ state_entering_handler,
+ NULL);
+ if (dst_enode)
+ eg->add_edge (src_enode, dst_enode, NULL, state_change (),
+ new signal_delivery_edge_info_t ());
+ }
+
+ const signal_state_machine &m_sm;
+ tree m_fndecl;
+};
+
+/* Return true if CALL is known to be unsafe to call from a signal handler. */
+
+static bool
+signal_unsafe_p (tree callee_fndecl)
+{
+ // TODO: maintain a list of known unsafe functions
+ if (is_named_call_p (callee_fndecl, "fprintf"))
+ return true;
+
+ return false;
+}
+
+/* Implementation of state_machine::on_stmt vfunc for signal_state_machine. */
+
+bool
+signal_state_machine::on_stmt (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt) const
+{
+ const state_t global_state = sm_ctxt->get_global_state ();
+ if (global_state == m_start)
+ {
+ if (const gcall *call = dyn_cast <const gcall *> (stmt))
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ if (is_named_call_p (callee_fndecl, "signal", call, 2))
+ {
+ tree handler = gimple_call_arg (call, 1);
+ if (TREE_CODE (handler) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (handler, 0)) == FUNCTION_DECL)
+ {
+ tree fndecl = TREE_OPERAND (handler, 0);
+ register_signal_handler rsh (*this, fndecl);
+ sm_ctxt->on_custom_transition (&rsh);
+ }
+ }
+ }
+ else if (global_state == m_in_signal_handler)
+ {
+ if (const gcall *call = dyn_cast <const gcall *> (stmt))
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ if (signal_unsafe_p (callee_fndecl))
+ sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler,
+ new signal_unsafe_call (*this, call,
+ callee_fndecl));
+ }
+
+ return false;
+}
+
+/* Implementation of state_machine::on_condition vfunc for
+ signal_state_machine. */
+
+void
+signal_state_machine::on_condition (sm_context *sm_ctxt ATTRIBUTE_UNUSED,
+ const supernode *node ATTRIBUTE_UNUSED,
+ const gimple *stmt ATTRIBUTE_UNUSED,
+ tree lhs ATTRIBUTE_UNUSED,
+ enum tree_code op ATTRIBUTE_UNUSED,
+ tree rhs ATTRIBUTE_UNUSED) const
+{
+ // Empty
+}
+
+bool
+signal_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const
+{
+ return true;
+}
+
+} // anonymous namespace
+
+/* Internal interface to this file. */
+
+state_machine *
+make_signal_state_machine (logger *logger)
+{
+ return new signal_state_machine (logger);
+}
@@ -115,6 +115,7 @@ make_checkers (auto_delete_vec <state_machine> &out, logger *logger)
out.safe_push (make_fileptr_state_machine (logger));
out.safe_push (make_taint_state_machine (logger));
out.safe_push (make_sensitive_state_machine (logger));
+ out.safe_push (make_signal_state_machine (logger));
/* We only attempt to run the pattern tests if it might have been manually
enabled (for DejaGnu purposes). */
@@ -184,6 +184,7 @@ extern state_machine *make_malloc_state_machine (logger *logger);
extern state_machine *make_fileptr_state_machine (logger *logger);
extern state_machine *make_taint_state_machine (logger *logger);
extern state_machine *make_sensitive_state_machine (logger *logger);
+extern state_machine *make_signal_state_machine (logger *logger);
extern state_machine *make_pattern_test_state_machine (logger *logger);
#endif /* GCC_ANALYZER_SM_H */
@@ -308,6 +308,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-null-dereference @gol
-Wno-analyzer-stale-setjmp-buffer @gol
-Wno-analyzer-tainted-array-index @gol
+-Wno-analyzer-unsafe-call-within-signal-handler @gol
-Wno-analyzer-use-after-free @gol
-Wno-analyzer-use-of-pointer-in-stale-stack-frame @gol
-Wno-analyzer-use-of-uninitialized-value @gol
@@ -406,6 +407,7 @@ Objective-C and Objective-C++ Dialects}.
-Wanalyzer-possible-null-dereference @gol
-Wanalyzer-stale-setjmp-buffer @gol
-Wanalyzer-tainted-array-index @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
-Wanalyzer-use-of-uninitialized-value @gol
@@ -6550,6 +6552,16 @@ This diagnostic warns for paths through the code in which a value
that could be under an attacker's control is used as the index
of an array access without being sanitized.
+@item -Wno-analyzer-unsafe-call-within-signal-handler
+@opindex Wanalyzer-unsafe-call-within-signal-handler
+@opindex Wno-analyzer-unsafe-call-within-signal-handler
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-unsafe-call-within-signal-handler} to disable it.
+
+This diagnostic warns for paths through the code in which a
+function known to be async-signal-unsafe (such as @code{fprintf}) is
+called from a signal handler.
+
@item -Wno-analyzer-use-after-free
@opindex Wanalyzer-use-after-free
@opindex Wno-analyzer-use-after-free
@@ -8262,6 +8274,7 @@ Enabling this option effectively enables the following warnings:
-Wanalyzer-null-argument @gol
-Wanalyzer-null-dereference @gol
-Wanalyzer-tainted-array-index @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-uninitialized-value @gol
-Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
new file mode 100644
@@ -0,0 +1,31 @@
+/* Example of a bad call within a signal handler.
+ 'handler' calls 'custom_logger' which calls 'fprintf', and 'fprintf' is
+ not allowed from a signal handler. */
+
+#include <stdio.h>
+#include <signal.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+int main(int argc, const char *argv)
+{
+ custom_logger("started");
+
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+ body_of_program();
+
+ custom_logger("stopped");
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,34 @@
+/* Example of a bad call within a signal handler.
+ 'handler' calls 'custom_logger' which calls 'fprintf', and 'fprintf' is
+ not allowed from a signal handler. */
+
+#include <stdio.h>
+#include <signal.h>
+
+extern void body_of_program(void);
+
+int logging = 1;
+
+void custom_logger(const char *msg)
+{
+ if (logging)
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+int main(int argc, const char *argv)
+{
+ custom_logger("started");
+
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+ body_of_program();
+
+ custom_logger("stopped");
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,23 @@
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+void test (void)
+{
+ void *ptr = malloc (1024);
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+ body_of_program();
+ free (ptr);
+}
new file mode 100644
@@ -0,0 +1,74 @@
+/* Verify how paths are printed for signal-handler diagnostics. */
+
+/* { dg-options "-fanalyzer -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+void test (void)
+{
+ void *ptr = malloc (1024);
+ signal(SIGINT, int_handler);
+ body_of_program();
+ free (ptr);
+}
+
+/* "call to 'fprintf' from within signal handler [CWE-479]". */
+/* { dg-begin-multiline-output "" }
+ NN | fprintf(stderr, "LOG: %s", msg);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'test': events 1-2
+ |
+ | NN | void test (void)
+ | | ^~~~
+ | | |
+ | | (1) entry to 'test'
+ |......
+ | NN | signal(SIGINT, int_handler);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) registering 'int_handler' as signal handler
+ |
+ event 3
+ |
+ |cc1:
+ | (3): later on, when the signal is delivered to the process
+ |
+ +--> 'int_handler': events 4-5
+ |
+ | NN | static void int_handler(int signum)
+ | | ^~~~~~~~~~~
+ | | |
+ | | (4) entry to 'int_handler'
+ | NN | {
+ | NN | custom_logger("got signal");
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (5) calling 'custom_logger' from 'int_handler'
+ |
+ +--> 'custom_logger': events 6-7
+ |
+ | NN | void custom_logger(const char *msg)
+ | | ^~~~~~~~~~~~~
+ | | |
+ | | (6) entry to 'custom_logger'
+ | NN | {
+ | NN | fprintf(stderr, "LOG: %s", msg);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (7) call to 'fprintf' from within signal handler
+ |
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,89 @@
+/* Verify how paths are printed for signal-handler diagnostics. */
+
+/* { dg-options "-fanalyzer -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+static void register_handler ()
+{
+ signal(SIGINT, int_handler);
+}
+
+void test (void)
+{
+ register_handler ();
+ body_of_program();
+}
+
+/* "call to 'fprintf' from within signal handler [CWE-479]". */
+/* { dg-begin-multiline-output "" }
+ NN | fprintf(stderr, "LOG: %s", msg);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'test': events 1-2
+ |
+ | NN | void test (void)
+ | | ^~~~
+ | | |
+ | | (1) entry to 'test'
+ | NN | {
+ | NN | register_handler ();
+ | | ~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) calling 'register_handler' from 'test'
+ |
+ +--> 'register_handler': events 3-4
+ |
+ | NN | static void register_handler ()
+ | | ^~~~~~~~~~~~~~~~
+ | | |
+ | | (3) entry to 'register_handler'
+ | NN | {
+ | NN | signal(SIGINT, int_handler);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (4) registering 'int_handler' as signal handler
+ |
+ event 5
+ |
+ |cc1:
+ | (5): later on, when the signal is delivered to the process
+ |
+ +--> 'int_handler': events 6-7
+ |
+ | NN | static void int_handler(int signum)
+ | | ^~~~~~~~~~~
+ | | |
+ | | (6) entry to 'int_handler'
+ | NN | {
+ | NN | custom_logger("got signal");
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (7) calling 'custom_logger' from 'int_handler'
+ |
+ +--> 'custom_logger': events 8-9
+ |
+ | NN | void custom_logger(const char *msg)
+ | | ^~~~~~~~~~~~~
+ | | |
+ | | (8) entry to 'custom_logger'
+ | NN | {
+ | NN | fprintf(stderr, "LOG: %s", msg);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (9) call to 'fprintf' from within signal handler
+ |
+ { dg-end-multiline-output "" } */