On Tue, Jul 02, 2024 at 05:06:14PM -0700, Andrii Nakryiko wrote: > > > Should it also check for ENDBR64? > > > > > Sure, I can add a check for endbr64 as well. endbr64 probably can be > used not just at function entry, is that right? So it might be another > case of false positive (which I think is ok, see below). Yeah, at least theoretically they could happen in the middle of a function for implementing C switch jump tables. > > > When compiled with -fcf-protection=branch, the first instruction of the > > > function will almost always be ENDBR64. I'm not sure about other > > > distros, but at least Fedora compiles its binaries like that. > > > > BTW, there are some cases (including leaf functions and some stack > > alignment sequences) where a "push %rbp" can happen inside a function. > > Then it would presumably add a bogus trace entry. Are such false > > positives ok? > > I think such cases should be rare. People mostly seem to trace user > function entry/exit, rarely if ever they trace something within the > function, except for USDT cases, where it will be a nop instruction > that they trace. > > In general, even with false positives, I think it's overwhelmingly > better to get correct entry stack trace 99.9% of the time, and in the > rest 0.01% cases it's fine having one extra bogus entry (but the rest > should still be correct), which should be easy for humans to recognize > and filter out, if necessary. Agreed, this is a definite improvement overall. BTW, soon there will be support for sframes instead of frame pointers, at which point these checks should only be done for the frame pointer case. -- Josh
