tracing: Add length protection to histogram string copies
authorSteven Rostedt (VMware) <rostedt@goodmis.org>
Sun, 14 Nov 2021 18:28:34 +0000 (13:28 -0500)
committerSteven Rostedt (VMware) <rostedt@goodmis.org>
Mon, 15 Nov 2021 02:21:08 +0000 (21:21 -0500)
The string copies to the histogram storage has a max size of 256 bytes
(defined by MAX_FILTER_STR_VAL). Only the string size of the event field
needs to be copied to the event storage, but no more than what is in the
event storage. Although nothing should be bigger than 256 bytes, there's
no protection against overwriting of the storage if one day there is.

Copy no more than the destination size, and enforce it.

Also had to turn MAX_FILTER_STR_VAL into an unsigned int, to keep the
min() comparison of the string sizes of comparable types.

Link: https://lore.kernel.org/all/CAHk-=wjREUihCGrtRBwfX47y_KrLCGjiq3t6QtoNJpmVrAEb1w@mail.gmail.com/
Link: https://lkml.kernel.org/r/20211114132834.183429a4@rorschach.local.home
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tom Zanussi <zanussi@kernel.org>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 63f84ae6b82b ("tracing/histogram: Do not copy the fixed-size char array field over the field size")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
include/linux/trace_events.h
kernel/trace/trace_events_hist.c

index 50453b2876155e3ae582bd7967542a1ae1c5a18a..2d167ac3452c58083517d9466e1fcbdcbb58709e 100644 (file)
@@ -673,7 +673,7 @@ struct trace_event_file {
 
 #define PERF_MAX_TRACE_SIZE    8192
 
-#define MAX_FILTER_STR_VAL     256     /* Should handle KSYM_SYMBOL_LEN */
+#define MAX_FILTER_STR_VAL     256U    /* Should handle KSYM_SYMBOL_LEN */
 
 enum event_trigger_type {
        ETT_NONE                = (0),
index 1475d7347fe0c10aac810a9de0c9a0d387361524..34afcaebd0e53d4b2148a88205a48f47e0a89993 100644 (file)
@@ -3026,8 +3026,10 @@ static inline void __update_field_vars(struct tracing_map_elt *elt,
                if (val->flags & HIST_FIELD_FL_STRING) {
                        char *str = elt_data->field_var_str[j++];
                        char *val_str = (char *)(uintptr_t)var_val;
+                       unsigned int size;
 
-                       strscpy(str, val_str, val->size);
+                       size = min(val->size, STR_VAR_LEN_MAX);
+                       strscpy(str, val_str, size);
                        var_val = (u64)(uintptr_t)str;
                }
                tracing_map_set_var(elt, var_idx, var_val);
@@ -4914,6 +4916,7 @@ static void hist_trigger_elt_update(struct hist_trigger_data *hist_data,
                        if (hist_field->flags & HIST_FIELD_FL_STRING) {
                                unsigned int str_start, var_str_idx, idx;
                                char *str, *val_str;
+                               unsigned int size;
 
                                str_start = hist_data->n_field_var_str +
                                        hist_data->n_save_var_str;
@@ -4922,7 +4925,9 @@ static void hist_trigger_elt_update(struct hist_trigger_data *hist_data,
 
                                str = elt_data->field_var_str[idx];
                                val_str = (char *)(uintptr_t)hist_val;
-                               strscpy(str, val_str, hist_field->size);
+
+                               size = min(hist_field->size, STR_VAR_LEN_MAX);
+                               strscpy(str, val_str, size);
 
                                hist_val = (u64)(uintptr_t)str;
                        }