Skip to content

Commit

Permalink
tracing: Add length protection to histogram string copies
Browse files Browse the repository at this point in the history
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/[email protected]

Cc: Ingo Molnar <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Tom Zanussi <[email protected]>
Reported-by: Linus Torvalds <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
Fixes: 63f84ae ("tracing/histogram: Do not copy the fixed-size char array field over the field size")
Signed-off-by: Steven Rostedt (VMware) <[email protected]>
  • Loading branch information
rostedt committed Nov 15, 2021
1 parent 1cab6bc commit 938aa33
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
2 changes: 1 addition & 1 deletion include/linux/trace_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
9 changes: 7 additions & 2 deletions kernel/trace/trace_events_hist.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down

0 comments on commit 938aa33

Please sign in to comment.