On Tue, 17 Sep 2024 10:55:39 +0100
Will Deacon <w...@kernel.org> wrote:
> The arm64 part looks good to me, although passing a partially-populated
> struct out of asm feels like it's going to cause us hard-to-debug
> problems down the line if any of those extra fields get used. How hard
> would it be to poison the unpopulated members of 'ftrace_regs'?
The purpose of creating ftrace_regs was to allow a partially populated
pt_regs to be sent around, as Thomas Gleixner and Peter Zijlstra were
against using pt_regs that were not fully populated. Hence, I created
"ftrace_regs" for this purpose.
ftrace_regs should never be accessed via its internal elements but only with
its accessor functions, as depending on the arch or functionality used, the
content of the structure should never be trusted. The accessor functions
will do all the verification needed.
I may add some compiler hacks to enforce this. Something like:
struct ftrace_regs {
void *nothing_to_see_here;
};
And then change the arch code to be something like:
// in arch/arm64/include/asm/ftrace.h:
struct arch_ftrace_regs {
/* x0 - x8 */
unsigned long regs[9];
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
unsigned long direct_tramp;
#else
unsigned long __unused;
#endif
unsigned long fp;
unsigned long lr;
unsigned long sp;
unsigned long pc;
};
#define get_arch_ftrace_regs(fregs) ((struct arch_ftrace_regs *)(fregs))
static __always_inline void
ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
unsigned long pc)
{
struct arch_ftrace_regs *afregs = get_ftrace_regs(fregs);
afregs->pc = pc;
}
-- Steve
