pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/41978?usp=email )
Change subject: logging: Avoid infinite loop logging inside logging path ......................................................................
logging: Avoid infinite loop logging inside logging path
If eg. gsmtap_log target is requested to log a message but the iofd wqueue is full, it may try to log an error message, which will create an infinite stack loop.
Change-Id: If3b8c0ee97537242162558fdd1c99f03b32af811 --- M include/osmocom/core/logging_internal.h M src/core/logging.c 2 files changed, 22 insertions(+), 4 deletions(-)
Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved
diff --git a/include/osmocom/core/logging_internal.h b/include/osmocom/core/logging_internal.h index 68d4c4b..0cc82a4 100644 --- a/include/osmocom/core/logging_internal.h +++ b/include/osmocom/core/logging_internal.h @@ -4,16 +4,25 @@ * @{ * \file logging_internal.h */
+#include <stdbool.h> #include <osmocom/core/logging.h> #include <osmocom/core/utils.h>
/* maximum length of the log string of a single log event (typically line) */ #define MAX_LOG_SIZE 4096
+struct log_thread_state { + /* Whether we are inside a code path to generate logging output: */ + bool logging_active; + /* Cache TID: */ + long int tid; +}; + extern void *tall_log_ctx; extern struct log_info *osmo_log_info; extern const struct value_string loglevel_strs[]; extern struct llist_head osmo_log_target_list; +extern __thread struct log_thread_state log_thread_state;
void assert_loginfo(const char *src);
diff --git a/src/core/logging.c b/src/core/logging.c index af5698a..3e60597 100644 --- a/src/core/logging.c +++ b/src/core/logging.c @@ -84,7 +84,7 @@ void *tall_log_ctx = NULL; LLIST_HEAD(osmo_log_target_list);
-static __thread long int logging_tid; +__thread struct log_thread_state log_thread_state;
#if (!EMBEDDED) /*! One global copy that contains the union of log levels for all targets @@ -606,9 +606,9 @@ OSMO_STRBUF_PRINTF(sb, " "); } if (target->print_tid) { - if (logging_tid == 0) - logging_tid = (long int)osmo_gettid(); - OSMO_STRBUF_PRINTF(sb, "%ld ", logging_tid); + if (log_thread_state.tid == 0) + log_thread_state.tid = (long int)osmo_gettid(); + OSMO_STRBUF_PRINTF(sb, "%ld ", log_thread_state.tid); } if (target->print_category) OSMO_STRBUF_PRINTF(sb, "%s%s%s%s ", @@ -743,6 +743,14 @@ { struct log_target *tar;
+ if (OSMO_UNLIKELY(log_thread_state.logging_active)) { + /* Avoid re-entrant logging: If logging to log target generates + * extra logging (eg. an error log line due to some wqueue being full), + * we may end up in an infinite loop. */ + return; + } + log_thread_state.logging_active = true; + subsys = map_subsys(subsys);
#if !defined(EMBEDDED) @@ -776,6 +784,7 @@ }
log_tgt_mutex_unlock(); + log_thread_state.logging_active = false; }
/*! logging function used by DEBUGP() macro