Hi, Sashiko made 2 comments.
https://sashiko.dev/#/patchset/20260522212627.12406-1-rosenp%40gmail.com On Fri, 22 May 2026 14:26:27 -0700 Rosen Penev <[email protected]> wrote: > > diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c > > index 0dd7927df22ac..196a7b0be99df 100644 > > --- a/kernel/trace/tracing_map.c > > +++ b/kernel/trace/tracing_map.c > [ ... ] > > @@ -125,32 +125,32 @@ u64 tracing_map_read_var_once(struct tracing_map_elt > > *elt, unsigned int i) > [ ... ] > > #define DEFINE_TRACING_MAP_CMP_FN(type) > > \ > > -static int tracing_map_cmp_##type(void *val_a, void *val_b) > > \ > > +static int tracing_map_cmp_##type(const void *val_a, const void *val_b) > > \ > > { \ > > - type a = (type)(*(u64 *)val_a); \ > > - type b = (type)(*(u64 *)val_b); \ > > + type a = (type)(*(const u64 *)val_a); \ > > + type b = (type)(*(const u64 *)val_b); \ > > \ > > return (a > b) ? 1 : ((a < b) ? -1 : 0); \ > > } > This is a pre-existing issue, but does unconditionally reading 8 bytes > via the u64 cast cause unaligned access exceptions on architectures that > do not support them? > Additionally, for fields near the end of the dynamically allocated elt->key > buffer, can this trigger KASAN slab-out-of-bounds reads? > Also, on big-endian architectures, reading a smaller integer as a 64-bit > value and casting it down extracts the least-significant bytes rather than > the correct field value. Could this result in completely incorrect sorting > for small types? Steve, it seems this comes from your commit 106f41f5a302 ("tracing: Have the histogram compare functions convert to u64 first"). I think neither of them is a problem, but could you check it? > > diff --git a/kernel/trace/tracing_map.h b/kernel/trace/tracing_map.h > > index 99c37eeebc161..ed64136782d80 100644 > > --- a/kernel/trace/tracing_map.h > > +++ b/kernel/trace/tracing_map.h > [ ... ] > > @@ -137,11 +137,11 @@ struct tracing_map_field { > > > > struct tracing_map_elt { > > struct tracing_map *map; > > - struct tracing_map_field *fields; > > atomic64_t *vars; > > bool *var_set; > > void *key; > > void *private_data; > > + struct tracing_map_field fields[]; > > }; > > Is it safe to add a flexible array member to struct tracing_map_elt when it > is embedded by value in the middle of struct track_data? > kernel/trace/trace_events_hist.c:struct track_data { > ... > struct tracing_map_elt elt; > ... > struct action_data *action_data; > struct hist_trigger_data *hist_data; > }; > Embedding a struct with a flexible array member in the middle of another > struct violates C standard constraints. Does this trigger compiler > warnings (such as -Wflex-array-member-not-at-end on modern compilers) or > break bounds computations for FORTIFY_SOURCE? Yeah, from this reason, this is is not acceptable. To fix this issue, we need to refactor the trace_events_hist.c, because track_data only uses tracing_map_elt as a placeholder of private_data. Thank you, -- Masami Hiramatsu (Google) <[email protected]>
