mbox series

[v2,0/7] HID: constify report descriptors

Message ID 20240803-hid-const-fixup-v2-0-f53d7a7b29d8@weissschuh.net (mailing list archive)
Headers show
Series HID: constify report descriptors | expand

Message

Thomas Weißschuh Aug. 3, 2024, 12:34 p.m. UTC
report descriptors are not meant to be constified outside of the HID
core. Enforce this invariant through the type system.

In addition it also allows the constification of static report
descriptors used by the report_fixup() callbacks.
This makes it clear to driver authors that the HID core will not modify
those reports and they can be reused for multiple devices.
Furthermore security is slightly improved as those reports are protected
against accidental or malicious modifications.

Patches "HID: constify hid_device::dev_rdesc" and
"HID: constify hid_device::rdesc" are very similar but the patch 
"HID: constify params and return value of fetch_item()" needs to be in
between.
It would however be possible to squash them together.

Only the cmedia driver has their static report descriptor constified as
a proof of concept as I'm the maintainer for that one, I didn't want to
spam all driver maintainers at this point.
I have patches for all other drivers that I'll submit after this series
is merged.

Note, this is only compile-tested.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v2:
- Drop RFC state
- Constify more of the internals of HID
- Drop now unnecessary patch introducing the variable "fixed_up"
- Link to v1: https://lore.kernel.org/r/20240730-hid-const-fixup-v1-0-f667f9a653ba@weissschuh.net

---
Thomas Weißschuh (7):
      HID: bpf: constify parameter rdesc of call_hid_bpf_rdesc_fixup()
      HID: constify parameter rdesc of hid_parse_report()
      HID: constify hid_device::rdesc
      HID: constify params and return value of fetch_item()
      HID: constify hid_device::dev_rdesc
      HID: change return type of report_fixup() to const
      HID: cmedia: constify fixed up report descriptor

 drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++----
 drivers/hid/hid-apple.c            |  2 +-
 drivers/hid/hid-asus.c             |  2 +-
 drivers/hid/hid-aureal.c           |  2 +-
 drivers/hid/hid-bigbenff.c         |  2 +-
 drivers/hid/hid-cherry.c           |  2 +-
 drivers/hid/hid-chicony.c          |  4 ++--
 drivers/hid/hid-cmedia.c           |  6 +++---
 drivers/hid/hid-core.c             | 14 +++++++-------
 drivers/hid/hid-corsair.c          |  4 ++--
 drivers/hid/hid-cougar.c           |  4 ++--
 drivers/hid/hid-cypress.c          |  2 +-
 drivers/hid/hid-dr.c               |  4 ++--
 drivers/hid/hid-elecom.c           |  2 +-
 drivers/hid/hid-gembird.c          |  2 +-
 drivers/hid/hid-glorious.c         |  2 +-
 drivers/hid/hid-holtek-kbd.c       |  2 +-
 drivers/hid/hid-holtek-mouse.c     |  4 ++--
 drivers/hid/hid-ite.c              |  2 +-
 drivers/hid/hid-keytouch.c         |  2 +-
 drivers/hid/hid-kye.c              |  2 +-
 drivers/hid/hid-lenovo.c           |  2 +-
 drivers/hid/hid-lg.c               |  2 +-
 drivers/hid/hid-logitech-hidpp.c   |  4 ++--
 drivers/hid/hid-macally.c          |  4 ++--
 drivers/hid/hid-magicmouse.c       |  4 ++--
 drivers/hid/hid-maltron.c          |  4 ++--
 drivers/hid/hid-microsoft.c        |  2 +-
 drivers/hid/hid-monterey.c         |  2 +-
 drivers/hid/hid-nti.c              |  2 +-
 drivers/hid/hid-ortek.c            |  2 +-
 drivers/hid/hid-petalynx.c         |  2 +-
 drivers/hid/hid-prodikeys.c        |  2 +-
 drivers/hid/hid-pxrc.c             |  4 ++--
 drivers/hid/hid-redragon.c         |  2 +-
 drivers/hid/hid-saitek.c           |  2 +-
 drivers/hid/hid-samsung.c          |  2 +-
 drivers/hid/hid-semitek.c          |  4 ++--
 drivers/hid/hid-sensor-hub.c       |  2 +-
 drivers/hid/hid-sigmamicro.c       |  4 ++--
 drivers/hid/hid-sony.c             |  2 +-
 drivers/hid/hid-steelseries.c      |  4 ++--
 drivers/hid/hid-sunplus.c          |  2 +-
 drivers/hid/hid-topre.c            |  4 ++--
 drivers/hid/hid-uclogic-core.c     |  2 +-
 drivers/hid/hid-viewsonic.c        |  4 ++--
 drivers/hid/hid-vrc2.c             |  4 ++--
 drivers/hid/hid-waltop.c           |  2 +-
 drivers/hid/hid-winwing.c          |  2 +-
 drivers/hid/hid-xiaomi.c           |  4 ++--
 drivers/hid/hid-zydacron.c         |  2 +-
 include/linux/hid.h                | 10 +++++-----
 include/linux/hid_bpf.h            |  2 +-
 53 files changed, 83 insertions(+), 85 deletions(-)
---
base-commit: c0ecd6388360d930440cc5554026818895199923
change-id: 20240730-hid-const-fixup-8b01cbda1b49

Best regards,

Comments

Benjamin Tissoires Aug. 27, 2024, 4:24 p.m. UTC | #1
On Aug 03 2024, Thomas Weißschuh wrote:
> report descriptors are not meant to be constified outside of the HID
> core. Enforce this invariant through the type system.
> 
> In addition it also allows the constification of static report
> descriptors used by the report_fixup() callbacks.
> This makes it clear to driver authors that the HID core will not modify
> those reports and they can be reused for multiple devices.
> Furthermore security is slightly improved as those reports are protected
> against accidental or malicious modifications.
> 
> Patches "HID: constify hid_device::dev_rdesc" and
> "HID: constify hid_device::rdesc" are very similar but the patch 
> "HID: constify params and return value of fetch_item()" needs to be in
> between.
> It would however be possible to squash them together.
> 
> Only the cmedia driver has their static report descriptor constified as
> a proof of concept as I'm the maintainer for that one, I didn't want to
> spam all driver maintainers at this point.
> I have patches for all other drivers that I'll submit after this series
> is merged.
> 
> Note, this is only compile-tested.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Thanks. Sorry for the delay, I'm only coming back of three weeks of vacations.

I've now applied the series to for-6.12/constify-rdesc on hid.git after
amending the series for latest master (small conflict in hid-cougar.c
and hid-multitouch.c needed a new hunk).

Cheers,
Benjamin

> ---
> Changes in v2:
> - Drop RFC state
> - Constify more of the internals of HID
> - Drop now unnecessary patch introducing the variable "fixed_up"
> - Link to v1: https://lore.kernel.org/r/20240730-hid-const-fixup-v1-0-f667f9a653ba@weissschuh.net
> 
> ---
> Thomas Weißschuh (7):
>       HID: bpf: constify parameter rdesc of call_hid_bpf_rdesc_fixup()
>       HID: constify parameter rdesc of hid_parse_report()
>       HID: constify hid_device::rdesc
>       HID: constify params and return value of fetch_item()
>       HID: constify hid_device::dev_rdesc
>       HID: change return type of report_fixup() to const
>       HID: cmedia: constify fixed up report descriptor
> 
>  drivers/hid/bpf/hid_bpf_dispatch.c |  6 ++----
>  drivers/hid/hid-apple.c            |  2 +-
>  drivers/hid/hid-asus.c             |  2 +-
>  drivers/hid/hid-aureal.c           |  2 +-
>  drivers/hid/hid-bigbenff.c         |  2 +-
>  drivers/hid/hid-cherry.c           |  2 +-
>  drivers/hid/hid-chicony.c          |  4 ++--
>  drivers/hid/hid-cmedia.c           |  6 +++---
>  drivers/hid/hid-core.c             | 14 +++++++-------
>  drivers/hid/hid-corsair.c          |  4 ++--
>  drivers/hid/hid-cougar.c           |  4 ++--
>  drivers/hid/hid-cypress.c          |  2 +-
>  drivers/hid/hid-dr.c               |  4 ++--
>  drivers/hid/hid-elecom.c           |  2 +-
>  drivers/hid/hid-gembird.c          |  2 +-
>  drivers/hid/hid-glorious.c         |  2 +-
>  drivers/hid/hid-holtek-kbd.c       |  2 +-
>  drivers/hid/hid-holtek-mouse.c     |  4 ++--
>  drivers/hid/hid-ite.c              |  2 +-
>  drivers/hid/hid-keytouch.c         |  2 +-
>  drivers/hid/hid-kye.c              |  2 +-
>  drivers/hid/hid-lenovo.c           |  2 +-
>  drivers/hid/hid-lg.c               |  2 +-
>  drivers/hid/hid-logitech-hidpp.c   |  4 ++--
>  drivers/hid/hid-macally.c          |  4 ++--
>  drivers/hid/hid-magicmouse.c       |  4 ++--
>  drivers/hid/hid-maltron.c          |  4 ++--
>  drivers/hid/hid-microsoft.c        |  2 +-
>  drivers/hid/hid-monterey.c         |  2 +-
>  drivers/hid/hid-nti.c              |  2 +-
>  drivers/hid/hid-ortek.c            |  2 +-
>  drivers/hid/hid-petalynx.c         |  2 +-
>  drivers/hid/hid-prodikeys.c        |  2 +-
>  drivers/hid/hid-pxrc.c             |  4 ++--
>  drivers/hid/hid-redragon.c         |  2 +-
>  drivers/hid/hid-saitek.c           |  2 +-
>  drivers/hid/hid-samsung.c          |  2 +-
>  drivers/hid/hid-semitek.c          |  4 ++--
>  drivers/hid/hid-sensor-hub.c       |  2 +-
>  drivers/hid/hid-sigmamicro.c       |  4 ++--
>  drivers/hid/hid-sony.c             |  2 +-
>  drivers/hid/hid-steelseries.c      |  4 ++--
>  drivers/hid/hid-sunplus.c          |  2 +-
>  drivers/hid/hid-topre.c            |  4 ++--
>  drivers/hid/hid-uclogic-core.c     |  2 +-
>  drivers/hid/hid-viewsonic.c        |  4 ++--
>  drivers/hid/hid-vrc2.c             |  4 ++--
>  drivers/hid/hid-waltop.c           |  2 +-
>  drivers/hid/hid-winwing.c          |  2 +-
>  drivers/hid/hid-xiaomi.c           |  4 ++--
>  drivers/hid/hid-zydacron.c         |  2 +-
>  include/linux/hid.h                | 10 +++++-----
>  include/linux/hid_bpf.h            |  2 +-
>  53 files changed, 83 insertions(+), 85 deletions(-)
> ---
> base-commit: c0ecd6388360d930440cc5554026818895199923
> change-id: 20240730-hid-const-fixup-8b01cbda1b49
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
>