All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] HID: storing pointers in 'hid_device_id::driver_data'
@ 2026-07-27 14:55 Pawel Zalewski (The Capable Hub)
  2026-07-27 14:55 ` [PATCH v3 1/8] HID: hid-input: use named initializer for 'hid_battery_quirks[]' Pawel Zalewski (The Capable Hub)
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Pawel Zalewski (The Capable Hub) @ 2026-07-27 14:55 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Jason Gerecke
  Cc: linux-kernel, linux-input, Pawel Zalewski (The Capable Hub)

The 'hid_device_id::driver_data' defined in <linux/device-id/hid.h> 
for the most part it is used for setting HID quirks and
treated as an integer value for storing metadata in the subsystem
drivers. But in a few instances it is used as a valid pointer to
dereference, namely in:
- hid-tmff
- wacom
 
Additionally this field can be set from the userspace via the
'hid_core::new_id_store' function - which is not memory safe and
could lead to OOB memory access in the above two cases.

This series is addressing the issue by making the usage of the field
consistent, all drives treat the value as an integer and if the
driver has a need for dereferencing a pointer - it stores and 
index to a table of pointers. The index is bounded by a named enum
and checked at runtime at the module level to avoid developer bugs.

Furthermore a user-input sanitizer was added in the 'hid_core' module
that restricts injecting arbitrary values for 'hid_device_id::driver_data'
that are not defined and matched within the driver already - which 
should be the case.

It was found that some modules that use the field do not use a named
initializers for the field - which is inconsistent with rest of the
modules within the HID subsystem. This is also addressed in the series
as part of the pre-clean up to increase clarity in patches 1-4. 

The actual implementation of the table of pointers approach 
can be found in patches 6-8. In case of wacom driver the sad thing
about it is that it requires a bit of macro magic for sanity. 

However, in doing so it was discovered that in the wacom driver
there is a 1:1 mapping between the 'driver_data' field and the 
product id - therefore patch 9 shows that the lookup can be
simplified as there is no need for using 'driver_data' at all
(it also does not require macro magic) - but it was left in a 
separate patch to show the two solution - happy to squash in v4
if this would be accepted.

The change also makes the code more portable on architecture 
like CHERI [1], where a pointer is replaced with a new primitive
(called the capability) at the architecture level and is twice as
wide as the greatest representable address, ie. for 64 bit address
space capabilities are 128 bits wide (the other 64 bits are used to
store meta-data relating to the 64 bit address). So you can not store 
valid pointers inside 'unsigned long' as effectively a different set of
instructions is being generated by the compiler based on the data-type
that was used in C (ie. capabilities have their own set of load/store
that also copy over the meta-data which are orthogonal to the load/store
instructions used for plain integers that would invalidate the meta-data).
There is slightly more detail to this, but the above is enough to
explain the motivation - the proposed changes make the code a bit
better even without considering CHERI at all - as it is more readable 
and type-safe.

The series was built and tested under QEMU (boots with relevant 
configs set to Y) on arm64. Additionally the operation of the input 
sanitizer was tested via the following:

```

modprobe wacom
echo "0x0003 0x056a 0x0001 0x00000" > /sys/bus/hid/drivers/wacom/new_id

echo "0x0003 0x056a 0x0000 0x000A1" > /sys/bus/hid/drivers/wacom/new_id

echo "0x0003 0x056a 0x0000 0x000A2" > /sys/bus/hid/drivers/wacom/new_id
-sh: echo: write error: Invalid argument

modprobe wacom
echo "0x0003 0x056a 0x0001 0x00000" > /sys/bus/hid/drivers/wacom/new_id

echo "0x0003 0x056a 0x0001 0x00001" > /sys/bus/hid/drivers/wacom/new_id
-sh: echo: write error: Invalid argument
echo "0x0003 0x056a 0x0001 0x00010" > /sys/bus/hid/drivers/wacom/new_id
-sh: echo: write error: Invalid argument
```

The 'hid-tmff' and other randomly picked modules were also tested as 
above. Furthermore the './hid-wacom.sh' test was run from the user space
and all tests (except the expected to-do's) passed as required.

This series is part of a larger effort led by Uwe [2]

[1] https://cheri-alliance.org/discover-cheri/
[2] https://lore.kernel.org/all/cover.1776429984.git.u.kleine-koenig@baylibre.com/ 

---
Changes in v3:
- rebased the series on top ov v7.2
- Link to v2: https://patch.msgid.link/20260610-mod-devicetable-hid_device_id-v2-0-a1d7473ccd9c@thegoodpenguin.co.uk

Changes in v2:
- Patches 1-4: No change
- Patches 5-8: Additional signed-off-by as patches were applied.
- Patches 9-12: Rewritten and new.
- Link to v1: https://patch.msgid.link/20260518-mod-devicetable-hid_device_id-v1-0-a08e3989c283@thegoodpenguin.co.uk

To: Jiri Kosina <jikos@kernel.org>
To: Benjamin Tissoires <bentiss@kernel.org>
To: Ping Cheng <ping.cheng@wacom.com>
To: Jason Gerecke <jason.gerecke@wacom.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Pawel Zalewski (The Capable Hub) (8):
      HID: hid-input: use named initializer for 'hid_battery_quirks[]'
      HID: hid-quirks: use named initializer in 'hid_quirks[]'
      HID: hid-asus: use named initializer for 'asus_devices[]'
      HID: i2c-hid-dmi-quirks: use named initializer for 'i2c_hid_elan_flipped_quirks[]'
      HID: hid-tmff: clean up usage of 'driver_data'
      HID: wacom: cleanup usage of 'driver_data'
      HID: hid-core: sanitize user input in 'new_id_store'
      HID: wacom: do not use 'driver_data'

 drivers/hid/hid-asus.c                   |  46 ++-
 drivers/hid/hid-core.c                   |  18 +
 drivers/hid/hid-input.c                  |  40 ++-
 drivers/hid/hid-quirks.c                 | 578 ++++++++++++++++++++-----------
 drivers/hid/hid-tmff.c                   |  48 ++-
 drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c |   2 +-
 drivers/hid/wacom.h                      |   1 +
 drivers/hid/wacom_sys.c                  |  25 +-
 drivers/hid/wacom_wac.c                  | 194 ++++++++++-
 drivers/hid/wacom_wac.h                  |   5 +
 10 files changed, 696 insertions(+), 261 deletions(-)
---
base-commit: 474951e8ec09da4e12d80503063793237ce121a5
change-id: 20260427-mod-devicetable-hid_device_id-7f30d877387c

Best regards,
--  
Pawel Zalewski (The Capable Hub) <pzalewski@thegoodpenguin.co.uk>


^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-07-27 15:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 14:55 [PATCH v3 0/8] HID: storing pointers in 'hid_device_id::driver_data' Pawel Zalewski (The Capable Hub)
2026-07-27 14:55 ` [PATCH v3 1/8] HID: hid-input: use named initializer for 'hid_battery_quirks[]' Pawel Zalewski (The Capable Hub)
2026-07-27 14:55 ` [PATCH v3 2/8] HID: hid-quirks: use named initializer in 'hid_quirks[]' Pawel Zalewski (The Capable Hub)
2026-07-27 14:55 ` [PATCH v3 3/8] HID: hid-asus: use named initializer for 'asus_devices[]' Pawel Zalewski (The Capable Hub)
2026-07-27 14:55 ` [PATCH v3 4/8] HID: i2c-hid-dmi-quirks: use named initializer for 'i2c_hid_elan_flipped_quirks[]' Pawel Zalewski (The Capable Hub)
2026-07-27 14:55 ` [PATCH v3 5/8] HID: hid-tmff: clean up usage of 'driver_data' Pawel Zalewski (The Capable Hub)
2026-07-27 15:08   ` sashiko-bot
2026-07-27 14:55 ` [PATCH v3 6/8] HID: wacom: cleanup " Pawel Zalewski (The Capable Hub)
2026-07-27 15:11   ` sashiko-bot
2026-07-27 14:55 ` [PATCH v3 7/8] HID: hid-core: sanitize user input in 'new_id_store' Pawel Zalewski (The Capable Hub)
2026-07-27 15:08   ` sashiko-bot
2026-07-27 14:55 ` [PATCH v3 8/8] HID: wacom: do not use 'driver_data' Pawel Zalewski (The Capable Hub)
2026-07-27 15:18   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.