-
Notifications
You must be signed in to change notification settings - Fork 7.4k
usb: fix unaligned access to interface and device descriptor #8495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
usb: fix unaligned access to interface and device descriptor #8495
Conversation
Fix unaligned access to interface and device descriptor. Signed-off-by: Johann Fischer <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #8495 +/- ##
=========================================
Coverage ? 64.61%
=========================================
Files ? 421
Lines ? 40296
Branches ? 6803
=========================================
Hits ? 26037
Misses ? 11126
Partials ? 3133 Continue to review full report at Codecov.
|
Hmm the structures are correctly marked as __packed, and the access is not done through a pointer of a different type, so GCC should generate unaligned access code itself. That looks like a compiler bug to me, that said, the workaround is correct. |
hid_cfg.if0_hid.subdesc[0].wDescriptorLength = | ||
sys_cpu_to_le16(report_desc_size); | ||
UNALIGNED_PUT(sys_cpu_to_le16(report_desc_size), | ||
&hid_cfg.if0_hid.subdesc[0].wDescriptorLength); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder could we use here sys_put_le16()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it now, should work, @SavinayDharmappa can you try it?
@@ -211,7 +211,8 @@ static struct usb_ep_cfg_data hid_ep_data[] = { | |||
|
|||
static void hid_interface_config(u8_t bInterfaceNumber) | |||
{ | |||
hid_cfg.if0.bInterfaceNumber = bInterfaceNumber; | |||
UNALIGNED_PUT(bInterfaceNumber, | |||
&hid_cfg.if0.bInterfaceNumber); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not need alignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be right, I have no knowledge about how Intel S1000 supports unaligned accesses
UNALIGNED_PUT((bInterfaceNumber + 1), | ||
&cdc_ecm_cfg.if1_0.bInterfaceNumber); | ||
UNALIGNED_PUT((bInterfaceNumber + 1), | ||
&cdc_ecm_cfg.if1_1.bInterfaceNumber); | ||
#ifdef CONFIG_USB_COMPOSITE_DEVICE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Use sys_put_le16() for unaligned access, this is refactored work of PR zephyrproject-rtos#8495 PR zephyrproject-rtos#11432 Signed-off-by: Andrei Emeltchenko <[email protected]>
Use sys_put_le16() for unaligned access, this is refactored work of PR #8495 PR #11432 Signed-off-by: Andrei Emeltchenko <[email protected]>
revised in #11619 |
Fix unaligned access to interface and device descriptor.
see #4661 (review)