| Message ID | 20240816100258.2159447-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] firmware: arm_ffa: Fix beyond size of field warning | expand |
On Fri, Aug 16, 2024 at 06:02:58PM +0800, Jinjie Ruan wrote: > An allmodconfig build of arm64 resulted in following warning: > > 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); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > In function ‘fortify_memcpy_chk’, > inlined from ‘ffa_msg_send_direct_req2’ at ./drivers/firmware/arm_ffa/driver.c:489:2: > ./linux-next/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); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Because ffa_msg_send_direct_req2() memcpy uuid_t and struct > ffa_send_direct_data2 data to unsigned long dst, the copy size is 2 or > or 14 unsigned long which beyond size of dst size, fix it by using a temp > array for memcpy. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/firmware/arm_ffa/driver.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c > index 1e3764852118..674fbe008ea6 100644 > --- a/drivers/firmware/arm_ffa/driver.c > +++ b/drivers/firmware/arm_ffa/driver.c > @@ -480,13 +480,23 @@ static int ffa_msg_send2(u16 src_id, u16 dst_id, void *buf, size_t sz) > static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid, > struct ffa_send_direct_data2 *data) > { > + unsigned long args_data[14]; > + unsigned long args_uuid[2]; > + unsigned long *data_ptr; > + > u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id); > ffa_value_t ret, args = { > .a0 = FFA_MSG_SEND_DIRECT_REQ2, .a1 = src_dst_ids, > }; > > - export_uuid((u8 *)&args.a2, uuid); > - memcpy(&args.a4, data, sizeof(*data)); > + memcpy(args_uuid, uuid, sizeof(uuid_t)); > + args.a2 = args_uuid[0]; > + args.a3 = args_uuid[1]; > + > + memcpy(args_data, data, sizeof(*data)); > + data_ptr = &args.a4; > + for (int i = 0; i < 14; i++) > + *data_ptr++ = args_data[i]; > So we end up with double copy for both uuid and ffa_send_direct_data2 ? This is not correct and not needed. Which toolchain are you using ? I got error only for memcpy which I forgot to push to -next, now fixed. It must appear in -next soon.
On 2024/8/19 18:35, Sudeep Holla wrote: > On Fri, Aug 16, 2024 at 06:02:58PM +0800, Jinjie Ruan wrote: >> An allmodconfig build of arm64 resulted in following warning: >> >> 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); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> In function ‘fortify_memcpy_chk’, >> inlined from ‘ffa_msg_send_direct_req2’ at ./drivers/firmware/arm_ffa/driver.c:489:2: >> ./linux-next/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); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> Because ffa_msg_send_direct_req2() memcpy uuid_t and struct >> ffa_send_direct_data2 data to unsigned long dst, the copy size is 2 or >> or 14 unsigned long which beyond size of dst size, fix it by using a temp >> array for memcpy. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> drivers/firmware/arm_ffa/driver.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c >> index 1e3764852118..674fbe008ea6 100644 >> --- a/drivers/firmware/arm_ffa/driver.c >> +++ b/drivers/firmware/arm_ffa/driver.c >> @@ -480,13 +480,23 @@ static int ffa_msg_send2(u16 src_id, u16 dst_id, void *buf, size_t sz) >> static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid, >> struct ffa_send_direct_data2 *data) >> { >> + unsigned long args_data[14]; >> + unsigned long args_uuid[2]; >> + unsigned long *data_ptr; >> + >> u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id); >> ffa_value_t ret, args = { >> .a0 = FFA_MSG_SEND_DIRECT_REQ2, .a1 = src_dst_ids, >> }; >> >> - export_uuid((u8 *)&args.a2, uuid); >> - memcpy(&args.a4, data, sizeof(*data)); >> + memcpy(args_uuid, uuid, sizeof(uuid_t)); >> + args.a2 = args_uuid[0]; >> + args.a3 = args_uuid[1]; >> + >> + memcpy(args_data, data, sizeof(*data)); >> + data_ptr = &args.a4; >> + for (int i = 0; i < 14; i++) >> + *data_ptr++ = args_data[i]; >> > > So we end up with double copy for both uuid and ffa_send_direct_data2 ? > This is not correct and not needed. > > Which toolchain are you using ? I got error only for memcpy which I forgot > to push to -next, now fixed. It must appear in -next soon. Use the newest linux-next and `make Image ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-`, the above compile error occurs. >
On 2024/8/19 19:01, Jinjie Ruan wrote: > > > On 2024/8/19 18:35, Sudeep Holla wrote: >> On Fri, Aug 16, 2024 at 06:02:58PM +0800, Jinjie Ruan wrote: >>> An allmodconfig build of arm64 resulted in following warning: >>> >>> 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); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> In function ‘fortify_memcpy_chk’, >>> inlined from ‘ffa_msg_send_direct_req2’ at ./drivers/firmware/arm_ffa/driver.c:489:2: >>> ./linux-next/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); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Because ffa_msg_send_direct_req2() memcpy uuid_t and struct >>> ffa_send_direct_data2 data to unsigned long dst, the copy size is 2 or >>> or 14 unsigned long which beyond size of dst size, fix it by using a temp >>> array for memcpy. >>> >>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >>> --- >>> drivers/firmware/arm_ffa/driver.c | 14 ++++++++++++-- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c >>> index 1e3764852118..674fbe008ea6 100644 >>> --- a/drivers/firmware/arm_ffa/driver.c >>> +++ b/drivers/firmware/arm_ffa/driver.c >>> @@ -480,13 +480,23 @@ static int ffa_msg_send2(u16 src_id, u16 dst_id, void *buf, size_t sz) >>> static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid, >>> struct ffa_send_direct_data2 *data) >>> { >>> + unsigned long args_data[14]; >>> + unsigned long args_uuid[2]; >>> + unsigned long *data_ptr; >>> + >>> u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id); >>> ffa_value_t ret, args = { >>> .a0 = FFA_MSG_SEND_DIRECT_REQ2, .a1 = src_dst_ids, >>> }; >>> >>> - export_uuid((u8 *)&args.a2, uuid); >>> - memcpy(&args.a4, data, sizeof(*data)); >>> + memcpy(args_uuid, uuid, sizeof(uuid_t)); >>> + args.a2 = args_uuid[0]; >>> + args.a3 = args_uuid[1]; >>> + >>> + memcpy(args_data, data, sizeof(*data)); >>> + data_ptr = &args.a4; >>> + for (int i = 0; i < 14; i++) >>> + *data_ptr++ = args_data[i]; >>> >> >> So we end up with double copy for both uuid and ffa_send_direct_data2 ? >> This is not correct and not needed. >> >> Which toolchain are you using ? I got error only for memcpy which I forgot >> to push to -next, now fixed. It must appear in -next soon. > > Use the newest linux-next and `make Image ARCH=arm64 > CROSS_COMPILE=aarch64-linux-gnu-`, the above compile error occurs. `make allmodconfig ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-` at first. > >>
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 1e3764852118..674fbe008ea6 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -480,13 +480,23 @@ static int ffa_msg_send2(u16 src_id, u16 dst_id, void *buf, size_t sz) static int ffa_msg_send_direct_req2(u16 src_id, u16 dst_id, const uuid_t *uuid, struct ffa_send_direct_data2 *data) { + unsigned long args_data[14]; + unsigned long args_uuid[2]; + unsigned long *data_ptr; + u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id); ffa_value_t ret, args = { .a0 = FFA_MSG_SEND_DIRECT_REQ2, .a1 = src_dst_ids, }; - export_uuid((u8 *)&args.a2, uuid); - memcpy(&args.a4, data, sizeof(*data)); + memcpy(args_uuid, uuid, sizeof(uuid_t)); + args.a2 = args_uuid[0]; + args.a3 = args_uuid[1]; + + memcpy(args_data, data, sizeof(*data)); + data_ptr = &args.a4; + for (int i = 0; i < 14; i++) + *data_ptr++ = args_data[i]; invoke_ffa_fn(args, &ret);
An allmodconfig build of arm64 resulted in following warning: 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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function ‘fortify_memcpy_chk’, inlined from ‘ffa_msg_send_direct_req2’ at ./drivers/firmware/arm_ffa/driver.c:489:2: ./linux-next/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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Because ffa_msg_send_direct_req2() memcpy uuid_t and struct ffa_send_direct_data2 data to unsigned long dst, the copy size is 2 or or 14 unsigned long which beyond size of dst size, fix it by using a temp array for memcpy. Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/firmware/arm_ffa/driver.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)