Skip to content

usb : class: hid: Fix fault due to unaligned access #11432

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 21, 2018
Merged

usb : class: hid: Fix fault due to unaligned access #11432

merged 1 commit into from
Nov 21, 2018

Conversation

SavinayDharmappa
Copy link
Contributor

patch fix fault due to unaligned access while setting hid
report size on xtensa platform.

Fixes #11266

Signed-off-by: Savinay Dharmappa [email protected]

rgundi
rgundi reviewed Nov 16, 2018
View reviewed changes
Copy link
Contributor

@rgundi rgundi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @finikorg @jfischer-phytec-iot Do you think this UNALIGNED_PUT should be used only for Xtensa platform? If it is generic, we can as well take out the __packed from the structure.

@finikorg
Copy link
Collaborator

@rgundi we cannot make descriptor table not packed. It is returned via USB requests.

@codecov-io
Copy link

codecov-io commented Nov 16, 2018 •
edited
Loading

Codecov Report

Merging #11432 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11432   +/-   ##
=======================================
  Coverage   48.41%   48.41%           
=======================================
  Files         270      270           
  Lines       42121    42121           
  Branches    10139    10139           
=======================================
  Hits        20394    20394           
  Misses      17645    17645           
  Partials     4082     4082

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57a6858...4af3183. Read the comment docs.

@rgundi
Copy link
Contributor

rgundi commented Nov 19, 2018

@rgundi we cannot make descriptor table not packed. It is returned via USB requests.

Thanks. So, should this UNALIGNED_PUT be used only for Xtensa platform?

@jfischer-no
Copy link
Collaborator

@SavinayDharmappa 😕, what is about #8495?

@SavinayDharmappa
Copy link
Contributor Author

@jfischer-phytec-iot I was not aware of #8495.

@finikorg
Copy link
Collaborator

@rgundi we cannot make descriptor table not packed. It is returned via USB requests.

Thanks. So, should this UNALIGNED_PUT be used only for Xtensa platform?

I think it is OK to have it for all platforms, maybe only it is better to use instead sys_put_le16().

@jfischer-no
Copy link
Collaborator

@SavinayDharmappa Do you like to take the changes from #8495 over?

@nashif
Copy link
Member

nashif commented Nov 20, 2018

remove "subsys: " from the commit messages...

@nashif
Copy link
Member

nashif commented Nov 20, 2018

@SavinayDharmappa Do you like to take the changes from #8495 over?

@SavinayDharmappa we should do that.

@SavinayDharmappa
Copy link
Contributor Author

@nashif @jfischer-phytec-iot #8495 have similar changes for other class of usb other than hid. I am not sure how would it affect the system.

@SavinayDharmappa
usb: class: hid: Fix fault due to unaligned access
4af3183 
patch fix fault due to unaligned access while setting hid
report size on xtensa platform.

Fixes zephyrproject-rtos#11266

Signed-off-by: Savinay Dharmappa <[email protected]>
@SavinayDharmappa SavinayDharmappa changed the title subsys :usb : class: hid: Fix fault due to unaligned access usb : class: hid: Fix fault due to unaligned access Nov 20, 2018
@finikorg
Copy link
Collaborator

@nashif @jfischer-phytec-iot #8495 have similar changes for other class of usb other than hid. I am not sure how would it affect the system.

Most of those are not needed since they use PUT_UNALIGNED with 1 byte. The only other change needed is modification of wTotalLength IIRC. So please use sys_put_le16() for those 2 changes.

@jfischer-no
Copy link
Collaborator

Most of those are not needed since they use PUT_UNALIGNED with 1 byte. The only other change needed is modification of wTotalLength IIRC. So please use sys_put_le16() for those 2 changes.

Let's merge it and discuss further in #8495.

@nashif nashif merged commit 0576f93 into zephyrproject-rtos:master Nov 21, 2018
finikorg added a commit to finikorg/zephyr that referenced this pull request Nov 23, 2018
@finikorg
usb: Fix unaligned access
350ad3c 
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]>
@finikorg finikorg mentioned this pull request Nov 23, 2018
Merged
andrewboie pushed a commit that referenced this pull request Nov 26, 2018
@finikorg @andrewboie
usb: Fix unaligned access
575bd0b 
Use sys_put_le16() for unaligned access, this is refactored work of
PR #8495
PR #11432

Signed-off-by: Andrei Emeltchenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgundi rgundi rgundi left review comments

@nashif nashif nashif approved these changes

@jfischer-no jfischer-no jfischer-no approved these changes

@finikorg finikorg Awaiting requested review from finikorg

@sathishkuttan sathishkuttan Awaiting requested review from sathishkuttan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

intel_s1000: usb_hid: load/store alignment exception
6 participants
@SavinayDharmappa @finikorg @codecov-io @rgundi @jfischer-no @nashif