From 8f241509d127f114f46c95769c0ec66e51be222b Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sun, 14 Jul 2019 17:48:01 -0700 Subject: charts: clamp %n length to buffer size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It appears I overlooked that %n returns the length that would have been printed regardless of the destination buffer size, not what was actually written. The man page is misleading here: n The number of characters written so far is stored into the integer pointed to by the corresponding argument. That argument shall be an int *, or variant whose size matches the (optionally) supplied integer length modifier. No argu‐ ment is converted. (This specifier is not supported by the bionic C library.) The behavior is undefined if the conver‐ sion specification includes any flags, a field width, or a precision. In testing, it isn't the count of what's actually written. It's oblivious of truncated output scenarios where the output buffer has been exhausted before reaching the %n. The man page should be clarified here. This commit does the simplest thing and simply clamps the length to the destination buffer - 1 (for the \0). %n is being used to avoid needing an strlen() in this somewhat hot path, but it might make sense to instead use the snprintf return value similarly clamped instead of %n since %n isn't doing what was expected. --- src/charts.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/charts.c b/src/charts.c index 0c0fa70..628b87a 100644 --- a/src/charts.c +++ b/src/charts.c @@ -590,6 +590,7 @@ static void draw_heirarchy_row(vwm_charts_t *charts, vwm_chart_t *chart, vmon_pr } else { /* we're a process having threads, suppress the wchan and state, as they will be displayed for the thread of same pid */ snprintf(str, sizeof(str), " %5i %n", proc->pid, &str_len); } + str_len = MIN(sizeof(str) - 1, str_len); str_width = XTextWidth(charts->chart_font, str, str_len); /* the process' comm label indented according to depth, followed with their respective argv's */ @@ -829,6 +830,7 @@ static void draw_chart(vwm_charts_t *charts, vwm_chart_t *chart, vmon_proc_t *pr /* only draw the \/\/\ and HZ if necessary */ if (chart->redraw_needed || charts->prev_sampling_interval != charts->sampling_interval) { snprintf(str, sizeof(str), "\\/\\/\\ %2iHz %n", (int)(charts->sampling_interval == INFINITY ? 0 : 1 / charts->sampling_interval), &str_len); + str_len = MIN(sizeof(str) - 1, str_len); XRenderFillRectangle(xserver->display, PictOpSrc, chart->text_picture, &chart_trans_color, 0, 0, /* dst x, y */ chart->visible_width, CHART_ROW_HEIGHT); /* dst w, h */ -- cgit v1.2.3