HID: wacom: Fix sibling detection broken by 345857b
Subsystems (2)
PATCH 1/1 HID: wacom: Fix sibling detection broken by 345857b
Commit 345857b included a change to the operation and location of the call to 'wacom_add_shared_data' in 'wacom_parse_and_register'. The modifications included moving it higher up so that it would occur before the call to 'wacom_retrieve_hid_descriptor'. This was done to prevent a crash that would have occured when the report containing tablet offsets was fed into the driver with 'wacom_hid_report_raw_event' (specifically: the various 'wacom_wac_*_report' functions were written with the assumption that they would only be called once tablet setup had completed; 'wacom_wac_pen_report' in particular dereferences 'shared' which wasn't yet allocated).
Moving the call to 'wacom_add_shared_data' effectively prevented the crash but also broke the sibiling detection code which assumes that the HID descriptor has been read and the various device_type flags set.
To fix this situation, we restore the original 'wacom_add_shared_data' operation and location and instead implement an alternative change that can also prevent the crash. Specifically, we notice that the report functions mentioned above expect to be called only for input reports. By adding a check, we can prevent feature reports (such as the offset report) from causing trouble.
--- drivers/hid/wacom_sys.c | 16 ++++++++-------- drivers/hid/wacom_wac.c | 10 ++++++++++ 2 files changed, 18 insertions(+), 8 deletions(-)
drivers/hid/wacom_sys.c +8 -8
@@ -740,6 +740,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)return retval; } +if (wacom_wac->features.device_type &WACOM_DEVICETYPE_TOUCH ) + wacom_wac->shared->touch = hdev; +else if (wacom_wac->features.device_type &WACOM_DEVICETYPE_PEN ) + wacom_wac->shared->pen = hdev; + out:mutex_unlock (&wacom_udev_list_lock);return retval;
@@ -2036,10 +2041,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)if (error) goto fail; - error =wacom_add_shared_data (hdev); -if (error) - goto fail; -/* * Bamboo Pad has a generic hid handling for the Pen, and we switch it * into debug mode for the touch part.
@@ -2080,10 +2081,9 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)wacom_update_name (wacom, wireless ?" (WL)" :"" ); -if (wacom_wac->features.device_type &WACOM_DEVICETYPE_TOUCH ) - wacom_wac->shared->touch = hdev; -else if (wacom_wac->features.device_type &WACOM_DEVICETYPE_PEN ) - wacom_wac->shared->pen = hdev; + error =wacom_add_shared_data (hdev); +if (error) + goto fail;if (!(features->device_type &WACOM_DEVICETYPE_WL_MONITOR ) && (features->quirks &WACOM_QUIRK_BATTERY )) {
drivers/hid/wacom_wac.c +10 -0
@@ -2187,6 +2187,16 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)wacom_report_events (hdev, report); +/* +* Non-input reports may be sent prior to the device being +* completely initialized. Since only their events need +* to be processed, exit after 'wacom_report_events' has +* been called to prevent potential crashes in the report- +* processing functions. +*/ +if (report->type !=HID_INPUT_REPORT ) +return ; +if (WACOM_PAD_FIELD (field)) {wacom_wac_pad_battery_report (hdev, report);if (wacom->wacom_wac.pad_input)
Signature · 1 line
2.11.0
Quoted 21 lines · Jason Gerecke
Commit 345857b included a change to the operation and location of the call to '
wacom_add_shared_data' in 'wacom_parse_and_register'. The modifications included moving it higher up so that it would occur before the call to 'wacom_retrieve_hid_descriptor'. This was done to prevent a crash that would have occured when the report containing tablet offsets was fed into the driver with 'wacom_hid_report_raw_event' (specifically: the various 'wacom_wac_*_report' functions were written with the assumption that they would only be called once tablet setup had completed; 'wacom_wac_pen_report' in particular dereferences 'shared' which wasn't yet allocated).Moving the call to '
wacom_add_shared_data' effectively prevented the crash but also broke the sibiling detection code which assumes that the HID descriptor has been read and the variousdevice_typeflags set.To fix this situation, we restore the original '
wacom_add_shared_data' operation and location and instead implement an alternative change that can also prevent the crash. Specifically, we notice that the report functions mentioned above expect to be called only for input reports. By adding a check, we can prevent feature reports (such as the offset report) from causing trouble.
Quoted 3 lines
Signed-off-by Jason Gerecke <[email protected]>Tested-by Ping Cheng <[email protected]>---
I agree this should go in v4.10 given that it introduces a regression for Wacom generic devices.
Cheers,
Benjamin
Quoted 69 lines
drivers/hid/wacom_sys.c | 16 ++++++++-------- drivers/hid/wacom_wac.c | 10 ++++++++++ 2 files changed, 18 insertions(+), 8 deletions(-)drivers/hid/wacom_sys.c +8
-8@@ -740,6 +740,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)return retval; } +if (wacom_wac->features.device_type &WACOM_DEVICETYPE_TOUCH ) + wacom_wac->shared->touch = hdev; +else if (wacom_wac->features.device_type &WACOM_DEVICETYPE_PEN ) + wacom_wac->shared->pen = hdev; + out:mutex_unlock (&wacom_udev_list_lock);return retval;@@ -2036,10 +2041,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)if (error) goto fail; - error =wacom_add_shared_data (hdev); -if (error) - goto fail; -/* * Bamboo Pad has a generic hid handling for the Pen, and we switch it * into debug mode for the touch part. @@ -2080,10 +2081,9 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)wacom_update_name (wacom, wireless ?" (WL)" :"" ); -if (wacom_wac->features.device_type &WACOM_DEVICETYPE_TOUCH ) - wacom_wac->shared->touch = hdev; -else if (wacom_wac->features.device_type &WACOM_DEVICETYPE_PEN ) - wacom_wac->shared->pen = hdev; + error =wacom_add_shared_data (hdev); +if (error) + goto fail;if (!(features->device_type &WACOM_DEVICETYPE_WL_MONITOR ) && (features->quirks &WACOM_QUIRK_BATTERY )) {drivers/hid/wacom_wac.c +10
-0@@ -2187,6 +2187,16 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)wacom_report_events (hdev, report); +/* +* Non-input reports may be sent prior to the device being +* completely initialized. Since only their events need +* to be processed, exit after 'wacom_report_events' has +* been called to prevent potential crashes in the report- +* processing functions. +*/ +if (report->type !=HID_INPUT_REPORT ) +return ; +if (WACOM_PAD_FIELD (field)) {wacom_wac_pad_battery_report (hdev, report);if (wacom->wacom_wac.pad_input)Signature · 1 line
2.11.0
Jiri Kosina
maintainerQuoted 21 lines · Jason Gerecke
Commit 345857b included a change to the operation and location of the call to '
wacom_add_shared_data' in 'wacom_parse_and_register'. The modifications included moving it higher up so that it would occur before the call to 'wacom_retrieve_hid_descriptor'. This was done to prevent a crash that would have occured when the report containing tablet offsets was fed into the driver with 'wacom_hid_report_raw_event' (specifically: the various 'wacom_wac_*_report' functions were written with the assumption that they would only be called once tablet setup had completed; 'wacom_wac_pen_report' in particular dereferences 'shared' which wasn't yet allocated).Moving the call to '
wacom_add_shared_data' effectively prevented the crash but also broke the sibiling detection code which assumes that the HID descriptor has been read and the variousdevice_typeflags set.To fix this situation, we restore the original '
wacom_add_shared_data' operation and location and instead implement an alternative change that can also prevent the crash. Specifically, we notice that the report functions mentioned above expect to be called only for input reports. By adding a check, we can prevent feature reports (such as the offset report) from causing trouble.
Quoted 2 lines
Signed-off-by Jason Gerecke <[email protected]>Tested-by Ping Cheng <[email protected]>
I've added Fixes: tag and applied to for-4.10/upstream-fixes.
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs