[PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[Charlie Jenkins]

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]

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@rivosinc.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
-- 
- Charlie

Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[Alexandre Ghiti]

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?

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@rivosinc.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

Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[Charlie Jenkins]

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
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@rivosinc.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

Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[Palmer Dabbelt]

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@rivosinc.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

Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[Alexandre Ghiti]

Hi Charlie,

On 25/02/2025 18:53, 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
> CONFIG_FUNCTION_TRACER=y
> CONFIG_FPROBE=y
> CONFIG_DYNAMIC_FTRACE=y
>
> I used gcc 14.2.0


I was missing FPROBE!

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


>
> - 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@rivosinc.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
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (for-next)
by Alexandre Ghiti <alexghiti@rivosinc.com>:

On Mon, 24 Feb 2025 18:42:21 -0800 you 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]
> 
> [...]

Here is the summary with links:
  - riscv: tracing: Fix __write_overflow_field in ftrace_partial_regs()
    https://git.kernel.org/riscv/c/bba547810c66

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html