[PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Gavin Shan]

Run into build warning caused by export_uuid() where the UUID's
length exceeds that of ffa_value_t::a2, as the following warning
messages indicate.

In function ‘fortify_memcpy_chk’,
inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
declared with attribute warning: detected write beyond size of field (1st parameter); \
maybe use struct_group()? [-Werror=attribute-warning]
571 |                         __write_overflow_field(p_size_field, size);
    |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fix it by not passing a plain buffer to memcpy() to avoid the overflow
and underflow warning, similar to what have been done to copy over the
struct ffa_send_direct_data2.

Fixes: aaef3bc98129 ("firmware: arm_ffa: Add support for FFA_MSG_SEND_DIRECT_{REQ,RESP}2")
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 drivers/firmware/arm_ffa/driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 4d231bc375e0..154777e89f04 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -485,7 +485,7 @@ static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid,
 		.a0 = FFA_MSG_SEND_DIRECT_REQ2, .a1 = src_dst_ids,
 	};
 
-	export_uuid((u8 *)&args.a2, uuid);
+	memcpy((void *)&args + offsetof(ffa_value_t, a2), uuid, sizeof(*uuid));
 	memcpy((void *)&args + offsetof(ffa_value_t, a4), data, sizeof(*data));
 
 	invoke_ffa_fn(args, &ret);
@@ -496,7 +496,7 @@ static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid,
 		return ffa_to_linux_errno((int)ret.a2);
 
 	if (ret.a0 == FFA_MSG_SEND_DIRECT_RESP2) {
-		memcpy(data, &ret.a4, sizeof(*data));
+		memcpy(data, (void *)&ret + offsetof(ffa_value_t, a4), sizeof(*data));
 		return 0;
 	}
 
-- 
2.45.2

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Sudeep Holla]

On Mon, Oct 14, 2024 at 10:47:24AM +1000, Gavin Shan wrote:
> Run into build warning caused by export_uuid() where the UUID's
> length exceeds that of ffa_value_t::a2, as the following warning
> messages indicate.
> 
> In function ‘fortify_memcpy_chk’,
> inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
> inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
> ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
> declared with attribute warning: detected write beyond size of field (1st parameter); \
> maybe use struct_group()? [-Werror=attribute-warning]
> 571 |                         __write_overflow_field(p_size_field, size);
>     |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fix it by not passing a plain buffer to memcpy() to avoid the overflow
> and underflow warning, similar to what have been done to copy over the
> struct ffa_send_direct_data2.
>

Are you observing this just on the upstream or -next as well? There is a
fix in the -next which I haven't sent to soc team yet, will do so soon.

-- 
Regards,
Sudeep

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Gavin Shan]

On 10/14/24 8:08 PM, Sudeep Holla wrote:
> On Mon, Oct 14, 2024 at 10:47:24AM +1000, Gavin Shan wrote:
>> Run into build warning caused by export_uuid() where the UUID's
>> length exceeds that of ffa_value_t::a2, as the following warning
>> messages indicate.
>>
>> In function ‘fortify_memcpy_chk’,
>> inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
>> inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
>> ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
>> declared with attribute warning: detected write beyond size of field (1st parameter); \
>> maybe use struct_group()? [-Werror=attribute-warning]
>> 571 |                         __write_overflow_field(p_size_field, size);
>>      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Fix it by not passing a plain buffer to memcpy() to avoid the overflow
>> and underflow warning, similar to what have been done to copy over the
>> struct ffa_send_direct_data2.
>>
> 
> Are you observing this just on the upstream or -next as well? There is a
> fix in the -next which I haven't sent to soc team yet, will do so soon.
> 

I just tried the upstream when the patch was posted. I just have a try with -next
and similar error exists.

[root@nvidia-grace-hopper-01 linux-next]# git remote -v
origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (fetch)
origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (push)
[root@nvidia-grace-hopper-01 linux-next]# make W=1 drivers/firmware/arm_ffa/driver.o
    :
In function ‘fortify_memcpy_chk’,
     inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:504:3:
./include/linux/fortify-string.h:580:25: error: call to ‘__read_overflow2_field’ declared with attribute warning: \
detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
   580 |                         __read_overflow2_field(q_size_field, size);
       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


Part of the changes included this patch is still needed by -next. Could you please
squeeze the changes to that one to be pulled?

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 8dd81db9b071..b14cbdae94e8 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -501,7 +501,7 @@ static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid,
                 return ffa_to_linux_errno((int)ret.a2);
  
         if (ret.a0 == FFA_MSG_SEND_DIRECT_RESP2) {
-               memcpy(data, &ret.a4, sizeof(*data));
+               memcpy(data, (void *)&ret + offsetof(ffa_value_t, a4), sizeof(*data));
                 return 0;
         }

Thanks,
Gavin

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Sudeep Holla]

On Mon, Oct 14, 2024 at 08:25:54PM +1000, Gavin Shan wrote:
> On 10/14/24 8:08 PM, Sudeep Holla wrote:
> > On Mon, Oct 14, 2024 at 10:47:24AM +1000, Gavin Shan wrote:
> > > Run into build warning caused by export_uuid() where the UUID's
> > > length exceeds that of ffa_value_t::a2, as the following warning
> > > messages indicate.
> > > 
> > > In function ‘fortify_memcpy_chk’,
> > > inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
> > > inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
> > > ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
> > > declared with attribute warning: detected write beyond size of field (1st parameter); \
> > > maybe use struct_group()? [-Werror=attribute-warning]
> > > 571 |                         __write_overflow_field(p_size_field, size);
> > >      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > Fix it by not passing a plain buffer to memcpy() to avoid the overflow
> > > and underflow warning, similar to what have been done to copy over the
> > > struct ffa_send_direct_data2.
> > > 
> > 
> > Are you observing this just on the upstream or -next as well? There is a
> > fix in the -next which I haven't sent to soc team yet, will do so soon.
> > 
> 
> I just tried the upstream when the patch was posted. I just have a try with -next
> and similar error exists.
> 
> [root@nvidia-grace-hopper-01 linux-next]# git remote -v
> origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (fetch)
> origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (push)
> [root@nvidia-grace-hopper-01 linux-next]# make W=1 drivers/firmware/arm_ffa/driver.o
>    :
> In function ‘fortify_memcpy_chk’,
>     inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:504:3:
> ./include/linux/fortify-string.h:580:25: error: call to ‘__read_overflow2_field’ declared with attribute warning: \
> detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   580 |                         __read_overflow2_field(q_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> 
> Part of the changes included this patch is still needed by -next. Could you please
> squeeze the changes to that one to be pulled?

Sure I can do that. Can you share the build command(specifically if any
extra warning are enabled) and the toolchain used ? I am unable to reproduce
it with clang 20.0.0, not sure if it my toolchain or build command/flags that
differs here.

--
Regards,
Sudeep

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Gavin Shan]

On 10/15/24 12:41 AM, Sudeep Holla wrote:
> On Mon, Oct 14, 2024 at 08:25:54PM +1000, Gavin Shan wrote:
>> On 10/14/24 8:08 PM, Sudeep Holla wrote:
>>> On Mon, Oct 14, 2024 at 10:47:24AM +1000, Gavin Shan wrote:
>>>> Run into build warning caused by export_uuid() where the UUID's
>>>> length exceeds that of ffa_value_t::a2, as the following warning
>>>> messages indicate.
>>>>
>>>> In function ‘fortify_memcpy_chk’,
>>>> inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
>>>> inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
>>>> ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
>>>> declared with attribute warning: detected write beyond size of field (1st parameter); \
>>>> maybe use struct_group()? [-Werror=attribute-warning]
>>>> 571 |                         __write_overflow_field(p_size_field, size);
>>>>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>
>>>> Fix it by not passing a plain buffer to memcpy() to avoid the overflow
>>>> and underflow warning, similar to what have been done to copy over the
>>>> struct ffa_send_direct_data2.
>>>>
>>>
>>> Are you observing this just on the upstream or -next as well? There is a
>>> fix in the -next which I haven't sent to soc team yet, will do so soon.
>>>
>>
>> I just tried the upstream when the patch was posted. I just have a try with -next
>> and similar error exists.
>>
>> [root@nvidia-grace-hopper-01 linux-next]# git remote -v
>> origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (fetch)
>> origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (push)
>> [root@nvidia-grace-hopper-01 linux-next]# make W=1 drivers/firmware/arm_ffa/driver.o
>>     :
>> In function ‘fortify_memcpy_chk’,
>>      inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:504:3:
>> ./include/linux/fortify-string.h:580:25: error: call to ‘__read_overflow2_field’ declared with attribute warning: \
>> detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>>    580 |                         __read_overflow2_field(q_size_field, size);
>>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>>
>> Part of the changes included this patch is still needed by -next. Could you please
>> squeeze the changes to that one to be pulled?
> 
> Sure I can do that. Can you share the build command(specifically if any
> extra warning are enabled) and the toolchain used ? I am unable to reproduce
> it with clang 20.0.0, not sure if it my toolchain or build command/flags that
> differs here.
> 

Thanks, Sudeep. I think what matters is 'W=1', which is translated to '-DKBUILD_EXTRA_WARN1'.
With KBUILD_EXTRA_WARN1 declared in linux-next, the warning is raised by __read_overflow2_field().

   static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid,
                                     struct ffa_send_direct_data2 *data)
   {
         :
         if (ret.a0 == FFA_MSG_SEND_DIRECT_RESP2) {
                 memcpy(data, &ret.a4, sizeof(*data));
                 return 0;
         }

         return -EINVAL;
   }

   memcpy
     __fortify_memcpy_chk
       fortify_memcpy_chk

   __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
                                            const size_t p_size,
                                            const size_t q_size,
                                            const size_t p_size_field,
                                            const size_t q_size_field,
                                            const u8 func)
   {
         if (__builtin_constant_p(size)) {
             :
                 /*
                  * Warn for source field over-read when building with W=1
                  * or when an over-write happened, so both can be fixed at
                  * the same time.
                  */
                 if ((IS_ENABLED(KBUILD_EXTRA_WARN1) ||                     <<< true:   KBUILD_EXTRA_WARN1 is declared
                      __compiletime_lessthan(p_size_field, size)) &&        <<< false:  sizeof(*data) == sizeof(*data)
                     __compiletime_lessthan(q_size_field, size))            <<< true:   sizeof(ret.a4) < sizeof(*data)
                         __read_overflow2_field(q_size_field, size);        <<< where the warning is reported
         }
         :
}

[root@nvidia-grace-hopper-01 linux-next]# cat /etc/redhat-release
Red Hat Enterprise Linux release 9.6 Beta (Plow)
[root@nvidia-grace-hopper-01 linux-next]# gcc --version
gcc (GCC) 11.5.0 20240719 (Red Hat 11.5.0-2)
[root@nvidia-grace-hopper-01 linux-next]# clang --version
clang version 18.1.8 (Red Hat, Inc. 18.1.8-3.el9)
Target: aarch64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Configuration file: /etc/clang/aarch64-redhat-linux-gnu-clang.cfg

[root@nvidia-grace-hopper-01 linux-next]# make V=1 W=1 drivers/firmware/arm_ffa/driver.o
    :
# CC [M]  drivers/firmware/arm_ffa/driver.o
   gcc -Wp,-MMD,drivers/firmware/arm_ffa/.driver.o.d -nostdinc -I./arch/arm64/include -I./arch/arm64/include/generated  -I./include -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT= -fmacro-prefix-map=./= -Werror -Wundef -DKBUILD_EXTRA_WARN1 -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -mgeneral-regs-only -DCONFIG_CC_HAS_K_CONSTRAINT=1 -Wno-psabi -mabi=lp64 -fno-asynchronous-unwind-tables -fno-unwind-tables -mbranch-protection=none -Wa,-march=armv8.5-a -DARM64_ASM_ARCH='"armv8.5-a"' -DKASAN_SHADOW_SCALE_SHIFT= -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-stack-clash-protection -fpatchable-function-entry=4,2 -fno-inline-functions-called-once -falign-functions=8 -fno-strict-overflow -fno-stack-check -fconserve-stack -Wall -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=2048 -Wno-main -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-overflow -Wno-array-bounds -Wno-alloc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wextra -Wunused -Wmissing-format-attribute -Wmissing-include-dirs -Wunused-const-variable -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negative-value -Wno-maybe-uninitialized -Wno-sign-compare -Wno-unused-parameter -g -mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=1992  -DMODULE  -DKBUILD_BASENAME='"driver"' -DKBUILD_MODNAME='"ffa_module"' -D__KBUILD_MODNAME=kmod_ffa_module -c -o drivers/firmware/arm_ffa/driver.o drivers/firmware/arm_ffa/driver.c
In file included from ./include/linux/string.h:390,
                  from ./include/linux/bitmap.h:13,
                  from ./include/linux/cpumask.h:12,
                  from ./arch/arm64/include/asm/cpufeature.h:27,
                  from ./arch/arm64/include/asm/ptrace.h:11,
                  from ./arch/arm64/include/asm/irqflags.h:9,
                  from ./include/linux/irqflags.h:18,
                  from ./include/linux/spinlock.h:59,
                  from ./include/linux/mmzone.h:8,
                  from ./include/linux/gfp.h:7,
                  from ./include/linux/slab.h:16,
                  from ./include/linux/resource_ext.h:11,
                  from ./include/linux/acpi.h:13,
                  from drivers/firmware/arm_ffa/driver.c:25:
In function ‘fortify_memcpy_chk’,
     inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:504:3:
./include/linux/fortify-string.h:580:25: error: call to ‘__read_overflow2_field’ declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
   580 |                         __read_overflow2_field(q_size_field, size);
       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
[root@nvidia-grace-hopper-01 linux-next]# echo $?
2

[root@nvidia-grace-hopper-01 linux-next]# make V=1 drivers/firmware/arm_ffa/driver.o
    :
# CC [M]  drivers/firmware/arm_ffa/driver.o
   gcc -Wp,-MMD,drivers/firmware/arm_ffa/.driver.o.d -nostdinc -I./arch/arm64/include -I./arch/arm64/include/generated  -I./include -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT= -fmacro-prefix-map=./= -Werror -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -mgeneral-regs-only -DCONFIG_CC_HAS_K_CONSTRAINT=1 -Wno-psabi -mabi=lp64 -fno-asynchronous-unwind-tables -fno-unwind-tables -mbranch-protection=none -Wa,-march=armv8.5-a -DARM64_ASM_ARCH='"armv8.5-a"' -DKASAN_SHADOW_SCALE_SHIFT= -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-stack-clash-protection -fpatchable-function-entry=4,2 -fno-inline-functions-called-once -falign-functions=8 -fno-strict-overflow -fno-stack-check -fconserve-stack -Wall -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=2048 -Wno-main -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-overflow -Wno-array-bounds -Wno-alloc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wextra -Wunused -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-packed-not-aligned -Wno-format-overflow -Wno-format-truncation -Wno-stringop-truncation -Wno-override-init -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negative-value -Wno-maybe-uninitialized -Wno-sign-compare -Wno-unused-parameter -g -mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=1992  -DMODULE  -DKBUILD_BASENAME='"driver"' -DKBUILD_MODNAME='"ffa_module"' -D__KBUILD_MODNAME=kmod_ffa_module -c -o drivers/firmware/arm_ffa/driver.o drivers/firmware/arm_ffa/driver.c
[root@nvidia-grace-hopper-01 linux-next]# echo $?
0

Thanks,
Gavin

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Sudeep Holla]

On Tue, Oct 15, 2024 at 09:43:32AM +1000, Gavin Shan wrote:
> On 10/15/24 12:41 AM, Sudeep Holla wrote:
> > On Mon, Oct 14, 2024 at 08:25:54PM +1000, Gavin Shan wrote:
> > > On 10/14/24 8:08 PM, Sudeep Holla wrote:
> > > > On Mon, Oct 14, 2024 at 10:47:24AM +1000, Gavin Shan wrote:
> > > > > Run into build warning caused by export_uuid() where the UUID's
> > > > > length exceeds that of ffa_value_t::a2, as the following warning
> > > > > messages indicate.
> > > > > 
> > > > > In function ‘fortify_memcpy_chk’,
> > > > > inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
> > > > > inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
> > > > > ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
> > > > > declared with attribute warning: detected write beyond size of field (1st parameter); \
> > > > > maybe use struct_group()? [-Werror=attribute-warning]
> > > > > 571 |                         __write_overflow_field(p_size_field, size);
> > > > >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > 
> > > > > Fix it by not passing a plain buffer to memcpy() to avoid the overflow
> > > > > and underflow warning, similar to what have been done to copy over the
> > > > > struct ffa_send_direct_data2.
> > > > > 
> > > > 
> > > > Are you observing this just on the upstream or -next as well? There is a
> > > > fix in the -next which I haven't sent to soc team yet, will do so soon.
> > > > 
> > > 
> > > I just tried the upstream when the patch was posted. I just have a try with -next
> > > and similar error exists.
> > > 
> > > [root@nvidia-grace-hopper-01 linux-next]# git remote -v
> > > origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (fetch)
> > > origin	git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (push)
> > > [root@nvidia-grace-hopper-01 linux-next]# make W=1 drivers/firmware/arm_ffa/driver.o
> > >     :
> > > In function ‘fortify_memcpy_chk’,
> > >      inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:504:3:
> > > ./include/linux/fortify-string.h:580:25: error: call to ‘__read_overflow2_field’ declared with attribute warning: \
> > > detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
> > >    580 |                         __read_overflow2_field(q_size_field, size);
> > >        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > cc1: all warnings being treated as errors
> > > 
> > > 
> > > Part of the changes included this patch is still needed by -next. Could you please
> > > squeeze the changes to that one to be pulled?
> > 
> > Sure I can do that. Can you share the build command(specifically if any
> > extra warning are enabled) and the toolchain used ? I am unable to reproduce
> > it with clang 20.0.0, not sure if it my toolchain or build command/flags that
> > differs here.
> > 
> 
> Thanks, Sudeep. I think what matters is 'W=1', which is translated to '-DKBUILD_EXTRA_WARN1'.
> With KBUILD_EXTRA_WARN1 declared in linux-next, the warning is raised by __read_overflow2_field().
> 

Sorry my bad, I got confused. I was not able to reproduce the write overflow
warning even if I revert the patch in the -next. I was seeing this warning,
just got confused. Thanks for the details. I have pushed this patch partially
as discussed since other warning is already addressed.

Regards,
Sudeep

-->8
From b0798838418abe996d9b618d341d865462264cbe Mon Sep 17 00:00:00 2001
From: Gavin Shan <gshan@redhat.com>
Date: Mon, 14 Oct 2024 10:47:24 +1000
Subject: [PATCH] firmware: arm_ffa: Avoid string-fortify warning caused by
 memcpy()

Copying from a 144 byte structure arm_smccc_1_2_regs at an offset of 32
into an 112 byte struct ffa_send_direct_data2 causes a compile-time warning:

 | In file included from drivers/firmware/arm_ffa/driver.c:25:
 | In function 'fortify_memcpy_chk',
 |    inlined from 'ffa_msg_send_direct_req2' at drivers/firmware/arm_ffa/driver.c:504:3:
 | include/linux/fortify-string.h:580:4: warning: call to '__read_overflow2_field'
 | 	declared with 'warning' attribute: detected read beyond size of field
 | 	(2nd parameter); maybe use struct_group()? [-Wattribute-warning]
 |                         __read_overflow2_field(q_size_field, size);

Fix it by not passing a plain buffer to memcpy() to avoid the overflow
warning.

Fixes: aaef3bc98129 ("firmware: arm_ffa: Add support for FFA_MSG_SEND_DIRECT_{REQ,RESP}2")
Signed-off-by: Gavin Shan <gshan@redhat.com>
Message-Id: <20241014004724.991353-1-gshan@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 8dd81db9b071..b14cbdae94e8 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -501,7 +501,7 @@ static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid,
 		return ffa_to_linux_errno((int)ret.a2);

 	if (ret.a0 == FFA_MSG_SEND_DIRECT_RESP2) {
-		memcpy(data, &ret.a4, sizeof(*data));
+		memcpy(data, (void *)&ret + offsetof(ffa_value_t, a4), sizeof(*data));
 		return 0;
 	}

--
2.34.1

Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

[Sudeep Holla]

On Mon, 14 Oct 2024 10:47:24 +1000, Gavin Shan wrote:
> Run into build warning caused by export_uuid() where the UUID's
> length exceeds that of ffa_value_t::a2, as the following warning
> messages indicate.
> 
> In function ‘fortify_memcpy_chk’,
> inlined from ‘export_uuid’ at ./include/linux/uuid.h:88:2,
> inlined from ‘ffa_msg_send_direct_req2’ at drivers/firmware/arm_ffa/driver.c:488:2:
> ./include/linux/fortify-string.h:571:25: error: call to ‘__write_overflow_field’ \
> declared with attribute warning: detected write beyond size of field (1st parameter); \
> maybe use struct_group()? [-Werror=attribute-warning]
> 571 |                         __write_overflow_field(p_size_field, size);
>     |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> [...]

With the modifications to the commit message and subject, applied to
sudeep.holla/linux (for-next/ffa/fixes) partially as required, thanks!

[1/1] firmware: arm_ffa: Fix warning caused by export_uuid()
      https://git.kernel.org/sudeep.holla/c/b0798838418a
--
Regards,
Sudeep