[ovs-dev] [PATCHv2] fatal-signal: Remove snprintf.

[Yifeng Sun]

Thanks, looks good to me.

Tested-by: Yifeng Sun <pkusunyifeng at gmail.com>

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>

On Wed, Mar 25, 2020 at 11:51 AM William Tu <u9012063 at gmail.com> wrote:

> Function snprintf is not async-signal-safe.  Replace it with
> our own implementation.  Example ovs-vswitchd.log output:
>   2020-03-25T01:08:19.673Z|00050|memory|INFO|handlers:2 ports:3
>   SIGSEGV detected, backtrace:
>   0x4872d9         <fatal_signal_handler+0x49>
>   0x7f4e2ab974b0   <killpg+0x40>
>   0x7f4e2ac5d74d   <__poll+0x2d>
>   0x531098         <time_poll+0x108>
>   0x51aefc         <poll_block+0x8c>
>   0x445ca9         <udpif_revalidator+0x289>
>   0x5056fd         <ovsthread_wrapper+0x7d>
>   0x7f4e2b65f6ba   <start_thread+0xca>
>   0x7f4e2ac6941d   <clone+0x6d>
>   0x0              <+0x0>
>
> Tested-at:
> https://travis-ci.org/github/williamtu/ovs-travis/builds/666875084
> Signed-off-by: William Tu <u9012063 at gmail.com>
> ---
> v2:
>   - avoid strcat overflow buffer, switch to use strncat
>   - some code refactor
>
> ---
>  lib/fatal-signal.c | 45 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 51cf628d994e..e033f1ec59ec 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -158,6 +158,23 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux),
> void (*cancel_cb)(void *aux),
>  }
>
>  #ifdef HAVE_UNWIND
> +/* Convert unsigned long long to string.  This is needed because
> + * using snprintf() is not async signal safe. */
> +static inline int
> +llong_to_hex_str(unsigned long long value, char *str)
> +{
> +    int i = 0, res;
> +
> +    if (value / 16 > 0) {
> +        i = llong_to_hex_str(value / 16, str);
> +    }
> +
> +    res = value % 16;
> +    str[i] = "0123456789abcdef"[res];
> +
> +    return i + 1;
> +}
> +
>  /* Send the backtrace buffer to monitor thread.
>   *
>   * Note that this runs in the signal handling context, any system
> @@ -192,20 +209,32 @@ send_backtrace_to_monitor(void) {
>                       dep * sizeof(struct unw_backtrace)));
>      } else {
>          /* Since there is no monitor daemon running, write backtrace
> -         * in current process.  This is not asyn-signal-safe due to
> -         * use of snprintf().
> +         * in current process.
>           */
>          char str[] = "SIGSEGV detected, backtrace:\n";
> +        char ip_str[16], offset_str[6];
> +        char _line[64];
> +        char *line = (char *)_line;
>
>          vlog_direct_write_to_log_file_unsafe(str);
>
>          for (int i = 0; i < dep; i++) {
> -            char line[64];
> -
> -            snprintf(line, 64, "0x%016"PRIxPTR" <%s+0x%"PRIxPTR">\n",
> -                     unw_bt[i].ip,
> -                     unw_bt[i].func,
> -                     unw_bt[i].offset);
> +            memset(line, 0, sizeof _line);
> +            memset(ip_str, ' ', sizeof ip_str);
> +            memset(offset_str, 0, sizeof offset_str);
> +            ip_str[sizeof(ip_str) - 1] = 0;
> +            offset_str[sizeof(offset_str) - 1] = 0;
> +
> +            llong_to_hex_str(unw_bt[i].ip, ip_str);
> +            llong_to_hex_str(unw_bt[i].offset, offset_str);
> +
> +            strcat(line, "0x");
> +            strcat(line, ip_str);
> +            strcat(line, "<");
> +            strncat(line, unw_bt[i].func, UNW_MAX_FUNCN);
> +            strcat(line, "+0x");
> +            strcat(line, offset_str);
> +            strcat(line, ">\n");
>              vlog_direct_write_to_log_file_unsafe(line);
>          }
>      }
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>