Skip to contentSkip to conversation

usb: functionfs: avoid memcpy() field overflow warning

PATCH In review Arnd Bergmann 3 participants 3 messages Jul 3, 2023 USB

lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 usb: functionfs: avoid memcpy() field overflow warning

From: Arnd Bergmann <[email protected]>

__ffs_func_bind_do_os_desc() copies both the CompatibleID and SubCompatibleID fields of the usb_ext_compat_desc structure into an array, which triggers a warning in the fortified memcpy():

In file included from drivers/usb/gadget/function/f_fs.c:17:
In file included from include/linux/string.h:254:
include/linux/fortify-string.h:592:4: error: call to '__read_overflow2_field' declared with 'warning' attribute: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]

                        __read_overflow2_field(q_size_field, size);

Usually we can avoid this by using a struct_group() inside of the structure definition, but this might cause problems in userspace since it is in a uapi header.

Just copy the two members individually.

Signed-off-by Arnd Bergmann <[email protected]>
---
 drivers/usb/gadget/function/f_fs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_fs.c +3 -2
index f41a385a5c421..b8f9e52e6db6b 100644
@@ -2933,8 +2933,9 @@ static int __ffs_func_bind_do_os_desc(enum ffs_os_desc_type type,
 		t = &func->function.os_desc_table[desc->bFirstInterfaceNumber];
 		t->if_id = func->interfaces_nums[desc->bFirstInterfaceNumber];
 		memcpy(t->os_desc->ext_compat_id, &desc->CompatibleID,
-		       ARRAY_SIZE(desc->CompatibleID) +
-		       ARRAY_SIZE(desc->SubCompatibleID));
+		       sizeof(desc->CompatibleID));
+		memcpy(t->os_desc->ext_compat_id + sizeof(desc->CompatibleID),
+			&desc->SubCompatibleID, sizeof(desc->SubCompatibleID));
 		length = sizeof(*desc);
 	}
 		break;
Signature · 1 line
2.39.2
maintainer
#1 lore raw
Quoted 6 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

__ffs_func_bind_do_os_desc() copies both the CompatibleID and SubCompatibleID fields of the usb_ext_compat_desc structure into an array, which triggers a warning in the fortified memcpy():

Quoted 8 lines

In file included from drivers/usb/gadget/function/f_fs.c:17:
In file included from include/linux/string.h:254:
include/linux/fortify-string.h:592:4: error: call to '__read_overflow2_field' declared with 'warning' attribute: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]

                        __read_overflow2_field(q_size_field, size);

Usually we can avoid this by using a struct_group() inside of the structure definition, but this might cause problems in userspace since it is in a uapi header.

We use this in other uapi .h files, what is unique about these fields that makes it so that they can not be used? Because it's not the last field?

thanks,

greg k-h

#2 lore raw
Quoted 12 lines · Greg Kroah-Hartman
Quoted 11 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

__ffs_func_bind_do_os_desc() copies both the CompatibleID and SubCompatibleID fields of the usb_ext_compat_desc structure into an array, which triggers a warning in the fortified memcpy():

In file included from drivers/usb/gadget/function/f_fs.c:17:
In file included from include/linux/string.h:254:
include/linux/fortify-string.h:592:4: error: call to '__read_overflow2_field' declared with 'warning' attribute: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]

                        __read_overflow2_field(q_size_field, size);
Quoted 7 lines
Quoted 3 lines

Usually we can avoid this by using a struct_group() inside of the structure definition, but this might cause problems in userspace since it is in a uapi header.

We use this in other uapi .h files, what is unique about these fields that makes it so that they can not be used? Because it's not the last field?

It's probably ok, and I was overly cautious. I'll send a new version after some more testing.

Arnd