On Wed, Mar 01, 2023 at 08:00:52PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Histogram values can not be strings, stacktraces, graphs, symbols,
> syscalls, or grouped in buckets or log. Give an error if a value is set to
> do so.
>
> Note, the histogram code was not prepared to handle these modifiers for
> histograms and caused a bug.
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: c6afad49d127f ("tracing: Add hist trigger 'sym' and 'sym-offset' modifiers")
> Reported-by: Mark Rutland <mark.rutland@xxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
Tested-by: Mark Rutland <mark.rutland@xxxxxxx>
I gave this a spin, and I see that the buckets modifier gerts rejected for
hitcount, but is usable for other values as it should be:
| # echo 'p:copy_to_user __arch_copy_to_user n=$arg3' >> /sys/kernel/tracing/kprobe_events
| # echo 'hist:keys=n:vals=hitcount.buckets=8:sort=hitcount' > /sys/kernel/tracing/events/kprobes/copy_to_user/trigger
| sh: write error: Invalid argument
| # echo 'hist:keys=n.buckets=8:vals=hitcount:sort=hitcount' > /sys/kernel/tracing/events/kprobes/copy_to_user/trigger
| # cat /sys/kernel/tracing/events/kprobes/copy_to_user/hist
| # event histogram
| #
| # trigger info: hist:keys=n.buckets=8:vals=hitcount:sort=hitcount:size=2048 [active]
| #
|
| { n: ~ 336-343 } hitcount: 1
| { n: ~ 16-23 } hitcount: 2
| { n: ~ 32-39 } hitcount: 2
| { n: ~ 832-839 } hitcount: 3
| { n: ~ 8-15 } hitcount: 3
| { n: ~ 128-135 } hitcount: 5
| { n: ~ 0-7 } hitcount: 57
|
| Totals:
| Hits: 73
| Entries: 7
| Dropped: 0
Thanks,
Mark.
> ---
> kernel/trace/trace_events_hist.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 89877a18f933..6e8ab726a7b5 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -4235,6 +4235,15 @@ static int __create_val_field(struct hist_trigger_data *hist_data,
> goto out;
> }
>
> + /* Some types cannot be a value */
> + if (hist_field->flags & (HIST_FIELD_FL_GRAPH | HIST_FIELD_FL_PERCENT |
> + HIST_FIELD_FL_BUCKET | HIST_FIELD_FL_LOG2 |
> + HIST_FIELD_FL_SYM | HIST_FIELD_FL_SYM_OFFSET |
> + HIST_FIELD_FL_SYSCALL | HIST_FIELD_FL_STACKTRACE)) {
> + hist_err(file->tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(field_str));
> + ret = -EINVAL;
> + }
> +
> hist_data->fields[val_idx] = hist_field;
>
> ++hist_data->n_vals;
> --
> 2.39.1
