linux-kernel - Re: [PATCH v14 04/19] function_graph: Replace fgraph_ret_regs with ftrace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240930145548.08c8f666@gandalf.local.home>
Date: Mon, 30 Sep 2024 14:55:48 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Will Deacon <will@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
 linux-arm-kernel@...ts.infradead.org, "Masami Hiramatsu (Google)"
 <mhiramat@...nel.org>, Florent Revest <revest@...omium.org>,
 linux-trace-kernel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
 Alexei Starovoitov <ast@...nel.org>, Jiri Olsa <jolsa@...nel.org>, Arnaldo
 Carvalho de Melo <acme@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v14 04/19] function_graph: Replace fgraph_ret_regs with
 ftrace_regs

On Tue, 17 Sep 2024 10:55:39 +0100
Will Deacon <will@...nel.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