linux-kernel - Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial

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: <mhng-2350fde8-5ca3-4770-94d0-26582bdfc0dd@palmer-ri-x1c9a>
Date: Tue, 25 Feb 2025 14:55:00 -0800 (PST)
From: Palmer Dabbelt <palmer@...belt.com>
To: Charlie Jenkins <charlie@...osinc.com>
CC: alex@...ti.fr, rostedt@...dmis.org, mhiramat@...nel.org,
  Mark Rutland <mark.rutland@....com>, Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
  oleg@...hat.com, linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
  linux-riscv@...ts.infradead.org
Subject:     Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

On Tue, 25 Feb 2025 09:53:27 PST (-0800), Charlie Jenkins wrote:
> On Tue, Feb 25, 2025 at 09:36:04AM +0100, Alexandre Ghiti wrote:
>> Hi Charlie,
>>
>> On 25/02/2025 03:42, Charlie Jenkins wrote:
>> > The size of &regs->a0 is unknown, causing the error:
>> >
>> > ../include/linux/fortify-string.h:571:25: warning: call to
>> > '__write_overflow_field' declared with attribute warning: detected write
>> > beyond size of field (1st parameter); maybe use struct_group()?
>> > [-Wattribute-warning]
>>
>>
>> I can't reproduce this warning with gcc and llvm, even when setting by hand
>> -Wattribute-warning when compiling bpf_trace.c (which is the user of
>> ftrace_partial_regs()).
>>
>> Which toolchain did you use?
>
> You need to have the configs:
> CONFIG_BPF_SYSCALL=y
> CONFIG_FORTIFY_SOURCE=y

I don't think I have any FORTIFY_SOURCE runs, unless they're on by 
default behind some other argument.  I'm kind of surprised this is the 
only bug we have, that usually trips up all sorts of stuff.

I'll go add some runs and see...

(I'm also going to just go pick this up, might take a little bit to show 
up.)

Thanks!

> CONFIG_FUNCTION_TRACER=y
> CONFIG_FPROBE=y
> CONFIG_DYNAMIC_FTRACE=y
>
> I used gcc 14.2.0
>
> - Charlie
>
>>
>> Thanks,
>>
>> Alex
>>
>>
>> >
>> > Fix this by wrapping the required registers in pt_regs with
>> > struct_group() and reference the group when doing the offending
>> > memcpy().
>> >
>> > Signed-off-by: Charlie Jenkins <charlie@...osinc.com>
>> > ---
>> >   arch/riscv/include/asm/ftrace.h |  2 +-
>> >   arch/riscv/include/asm/ptrace.h | 18 ++++++++++--------
>> >   2 files changed, 11 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
>> > index c4721ce44ca474654b37b3d51bc0a63d46bc1eff..ec6db1162021fbf4fa48fc87e7984266040aa7d9 100644
>> > --- a/arch/riscv/include/asm/ftrace.h
>> > +++ b/arch/riscv/include/asm/ftrace.h
>> > @@ -207,7 +207,7 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
>> >   {
>> >   	struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
>> > -	memcpy(&regs->a0, afregs->args, sizeof(afregs->args));
>> > +	memcpy(&regs->a_regs, afregs->args, sizeof(afregs->args));
>> >   	regs->epc = afregs->epc;
>> >   	regs->ra = afregs->ra;
>> >   	regs->sp = afregs->sp;
>> > diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
>> > index b5b0adcc85c18e15c156de11172a5d7f03ada037..2910231977cb71dac3cc42f2dc32590284204057 100644
>> > --- a/arch/riscv/include/asm/ptrace.h
>> > +++ b/arch/riscv/include/asm/ptrace.h
>> > @@ -23,14 +23,16 @@ struct pt_regs {
>> >   	unsigned long t2;
>> >   	unsigned long s0;
>> >   	unsigned long s1;
>> > -	unsigned long a0;
>> > -	unsigned long a1;
>> > -	unsigned long a2;
>> > -	unsigned long a3;
>> > -	unsigned long a4;
>> > -	unsigned long a5;
>> > -	unsigned long a6;
>> > -	unsigned long a7;
>> > +	struct_group(a_regs,
>> > +		unsigned long a0;
>> > +		unsigned long a1;
>> > +		unsigned long a2;
>> > +		unsigned long a3;
>> > +		unsigned long a4;
>> > +		unsigned long a5;
>> > +		unsigned long a6;
>> > +		unsigned long a7;
>> > +	);
>> >   	unsigned long s2;
>> >   	unsigned long s3;
>> >   	unsigned long s4;
>> >
>> > ---
>> > base-commit: d082ecbc71e9e0bf49883ee4afd435a77a5101b6
>> > change-id: 20250224-fix_ftrace_partial_regs-eddaf4a7e5ed