Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Out-of-bounds read and potential unaligned access when parsing the dynamically sized cpumask trace field with CPU_AND. - [Medium] Missing return value check for tep_get_field_val() leaves dst_cpu uninitialized, leading to out-of-bounds memory accesses. --
commit ff03318823c0d1c72fb9d5969112afecabf91462 Author: Valentin Schneider <[email protected]> rtla/osnoise: Record IPI count in osnoise top This commit leverages the ipi_send_cpu and ipi_send_cpumask trace events to record the count of IPIs sent to monitored CPUs. It adds these statistics to the osnoise top output, enabled via the new --ipi command line option. > diff --git a/tools/tracing/rtla/src/osnoise_top.c > b/tools/tracing/rtla/src/osnoise_top.c > index 512a6299cb018..7b5ae5336cf08 100644 > --- a/tools/tracing/rtla/src/osnoise_top.c > +++ b/tools/tracing/rtla/src/osnoise_top.c [ ... ] > @@ -275,12 +282,93 @@ osnoise_top_apply_config(struct osnoise_tool *tool) > return -1; > } > > +static void account_ipi(struct osnoise_tool *tool, unsigned long long > dst_cpu) > +{ > + struct osnoise_top_cpu *cpu_data; > + struct osnoise_top_data *data; > + unsigned long long inc = 1; > + > + data = tool->data; > + cpu_data = &data->cpu_data[dst_cpu]; > + > + update_sum(&cpu_data->ipi_count, &inc); > +} > + > +/* > + * osnoise_ipi_cpu_handler - this is the handler for single CPU IPI events. > + */ > +static int > +osnoise_ipi_cpu_handler(struct trace_seq *s, struct tep_record *record, > + struct tep_event *event, void *context) > +{ > + struct osnoise_tool *tool; > + struct osnoise_params *params; > + unsigned long long dst_cpu; > + struct trace_instance *trace = context; > + > + tool = container_of(trace, struct osnoise_tool, trace); > + params = to_osnoise_params(tool->params); > + > + tep_get_field_val(s, event, "cpu", record, &dst_cpu, 1); > + > + if (CPU_ISSET(dst_cpu, ¶ms->common.monitored_cpus)) > + account_ipi(tool, dst_cpu); [Severity: Medium] If tep_get_field_val() fails to extract the field (for instance, due to a trace format mismatch), the return value is ignored and dst_cpu is left uninitialized. Could this result in an out-of-bounds read inside the CPU_ISSET() macro, or an out-of-bounds heap write when dst_cpu is passed to account_ipi() and used as an index for data->cpu_data[dst_cpu]? > + > + return 0; > +} > + > +static cpu_set_t cpumask_tmp_cpus; > + > +/* > + * osnoise_ipi_cpumask_handler - this is the handler for broadcasted IPI > events. > + */ > +static int > +osnoise_ipi_cpumask_handler(struct trace_seq *s, struct tep_record *record, > + struct tep_event *event, void *context) > +{ > + struct trace_instance *trace = context; > + struct osnoise_tool *tool; > + struct osnoise_params *params; > + struct tep_format_field *field; > + cpu_set_t *event_cpus; > + int len; > + > + tool = container_of(trace, struct osnoise_tool, trace); > + params = to_osnoise_params(tool->params); > + > + field = tep_find_field(event, "cpumask"); > + if (!field) > + return 0; > + > + event_cpus = tep_get_field_raw(s, event, "cpumask", record, &len, 1); > + if (!event_cpus) { > + err_msg("Failed to get cpumask field\n"); > + return 0; > + } > + > + CPU_AND(&cpumask_tmp_cpus, event_cpus, ¶ms->common.monitored_cpus); [Severity: Medium] The event_cpus pointer returned by tep_get_field_raw() points to dynamically sized raw bytes according to the len parameter. The glibc CPU_AND() macro assumes both source arguments are full cpu_set_t objects and unconditionally operates on 128 bytes. Does this cause an out-of-bounds read and potential unaligned access when parsing the trace field with CPU_AND()? > + > + /* > + * Computing the mask weight is overkill but there is no leaner option > + * provided by glibc, e.g cpumask_first() or somesuch. > + */ > + if (CPU_COUNT(&cpumask_tmp_cpus)) { > + for (int cpu = 0; cpu < nr_cpus; cpu++) { > + if (CPU_ISSET(cpu, &cpumask_tmp_cpus)) > + account_ipi(tool, cpu); > + } > + } > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
