Hi Naveed, Adding Serhei to CC to double check when the ebl and abi can be different and whether frame_nregs can ever be smaller than dwarf_to_perf_len.
On Mon, Jun 22, 2026 at 03:26:00PM +0530, Naveed Khan wrote: > x86_sample_perf_regs_mapping selects dwarf_to_perf from one of two > tables, regs_i386[] (9 entries) or regs_x86_64[] (17 entries), depending > on the sample ABI. The final mapping loop was bounded by > ebl->frame_nregs and indexed dwarf_to_perf[i]. For the x86_64 backend > frame_nregs is 17, so when a sample carries PERF_SAMPLE_REGS_ABI_32 > (is_abi32 true) the loop reads regs_i386[9..16], eight ints past the end > of the 9-element array. The resulting out-of-bounds values are then used > to index perf_to_regs[]. > > The abi value originates from a perf sample and is passed unvalidated > through the public dwflst_perf_sample_getframes() API; it is never > checked against the backend ELF class. Commit a532f8d hardened the > perf_to_regs[] write side but left this read overflow in place. > > Bound the loop by the length of the selected table so it never indexes > past the array in use. > > * backends/x86_initreg_sample.c (x86_sample_perf_regs_mapping): > Compute dwarf_to_perf_len for the selected table and bound the > mapping loop by it. The analysis and fix look correct. But I have a small performance nitpick. > Signed-off-by: Naveed Khan <[email protected]> > --- > diff --git a/backends/x86_initreg_sample.c b/backends/x86_initreg_sample.c > index 07f0f56..eed2943 100644 > --- a/backends/x86_initreg_sample.c > +++ b/backends/x86_initreg_sample.c > @@ -63,6 +63,13 @@ x86_sample_perf_regs_mapping (Ebl *ebl, > 16/*r8 after flags+segment*/, 17, 18, 19, > 20, 21, 22, 23, > 8/*ip*/}; > const int *dwarf_to_perf = is_abi32 ? regs_i386 : regs_x86_64; > + /* The two tables have different lengths; the loop below must not index > + dwarf_to_perf beyond the selected one. This matters when abi does > + not match the backend (e.g. an x86_64 ebl, frame_nregs == 17, > + handling an ABI_32 sample where regs_i386 only has 9 entries). */ > + size_t dwarf_to_perf_len = is_abi32 ? > + sizeof (regs_i386) / sizeof (regs_i386[0]) : > + sizeof (regs_x86_64) / sizeof (regs_x86_64[0]); > > /* Count bits and allocate regs_mapping: */ > int j, k, count; uint64_t bit; > @@ -106,7 +113,7 @@ x86_sample_perf_regs_mapping (Ebl *ebl, > > /* Locations of perf_regs in the dwarf_regs array, according to > perf_regs_mask and perf_to_regs[]: */ > - for (size_t i = 0; i < ebl->frame_nregs; i++) > + for (size_t i = 0; i < ebl->frame_nregs && i < dwarf_to_perf_len; i++) > { > k = dwarf_to_perf[i]; > j = perf_to_regs[k]; Looks like the compiler cannot easily see that the loop is bounded by MIN (ebl->frame_nregs, dwarf_to_perf_len). Maybe do something like: size_t max_reg = MIN (ebl->frame_nregs, dwarf_to_perf_len); for (size_t i = 0; i < max_reg; i++) Or maybe dwarf_to_perf_len never is bigger than ebl->frame_nregs? Then we could just assert that and just loop till dwarf_to_perf_len. Thanks, Mark
