Re: [PATCH] HID: wacom: Ignore attempts to overwrite the touch_max value from HID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 16 Feb 2021, Jason Gerecke wrote:

> The `wacom_feature_mapping` function is careful to only set the the
> touch_max value a single time, but this care does not extend to the
> `wacom_wac_finger_event` function. In particular, if a device sends
> multiple HID_DG_CONTACTMAX items in a single feature report, the
> driver will end up retaining the value of last item.
> 
> The HID descriptor for the Cintiq Companion 2 does exactly this. It
> incorrectly sets a "Report Count" of 2, which will cause the driver
> to process two HID_DG_CONTACTCOUNT items. The first item has the actual
> count, while the second item should have been declared as a constant
> zero. The constant zero is the value the driver ends up using, however,
> since it is the last HID_DG_CONTACTCOUNT in the report.
> 
>     Report ID (16),
>     Usage (Contact Count Maximum),  ; Contact count maximum (55h, static value)
>     Report Count (2),
>     Logical Maximum (10),
>     Feature (Variable),
> 
> To address this, we add a check that the touch_max is not already set
> within the `wacom_wac_finger_event` function that processes the
> HID_DG_TOUCHMAX item. We emit a warning if the value is set and ignore
> the updated value.
> 
> This could potentially cause problems if there is a tablet which has
> a similar issue but requires the last item to be used. This is unlikely,
> however, since it would have to have a different non-zero value for
> HID_DG_CONTACTMAX earlier in the same report, which makes no sense
> except in the case of a firmware bug. Note that cases where the
> HID_DG_CONTACTMAX items are in different reports is already handled
> (and similarly ignored) by `wacom_feature_mapping` as mentioned above.
> 
> Link: https://github.com/linuxwacom/input-wacom/issues/223
> Fixes: 184eccd40389 ("HID: wacom: generic: read HID_DG_CONTACTMAX from any feature report")
> Signed-off-by: Jason Gerecke <jason.gerecke@xxxxxxxxx>
> CC: stable@xxxxxxxxxxxxxxx
> ---
>  drivers/hid/wacom_wac.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 1bd0eb71559c..44d715c12f6a 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -2600,7 +2600,12 @@ static void wacom_wac_finger_event(struct hid_device *hdev,
>  		wacom_wac->is_invalid_bt_frame = !value;
>  		return;
>  	case HID_DG_CONTACTMAX:
> -		features->touch_max = value;
> +		if (!features->touch_max) {
> +			features->touch_max = value;
> +		} else {
> +			hid_warn(hdev, "%s: ignoring attempt to overwrite non-zero touch_max "
> +				 "%d -> %d\n", __func__, features->touch_max, value);
> +		}
>  		return;

Applied, thanks Jason.

-- 
Jiri Kosina
SUSE Labs




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux