linux-kernel - Re: [PATCH] firmware: arm_ffa: Fix warning caused by export

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: <2ea2b741-3abd-4fe1-b622-b6a4a3c2a92b@redhat.com>
Date: Mon, 14 Oct 2024 20:25:54 +1000
From: Gavin Shan <gshan@...hat.com>
To: Sudeep Holla <sudeep.holla@....com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 shan.gavin@...il.com
Subject: Re: [PATCH] firmware: arm_ffa: Fix warning caused by export_uuid()

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