Re: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report
From: sashiko-bot@kernel.org
Date: 2026-07-21 14:41:30
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unconditional `field->hidinput` dereference and tablet mode state corruption due to unconstrained `.event` callback. - [High] The Kconfig symbol `INPUT_AMD_SFH_TABLETMODE` is missing a dependency on `HID`. --
--- Patch [2]: [PATCH 2/2] amd-sfh-tabletmode: interpret sfh tablet mode hid report ---commit 981d61514e0f554ad31b2f14f7fb391c059df340
Author: Helge Bahmann [off-list ref]
amd-sfh-tabletmode: interpret sfh tablet mode hid report
Add a driver that hooks into the tablet mode hid reports
generated by the amd-sfh-hid driver. This enables tablet
mode switching on e.g. Asus Vivobook devices.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 94a753fcb64fa..5a1dacd6f7b9c 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig@@ -1003,4 +1003,15 @@ config INPUT_STPMIC1_ONKEY To compile this driver as a module, choose M here: the module will be called stpmic1_onkey. +config INPUT_AMD_SFH_TABLETMODE + tristate "AMD sensor fusion hub tablet mode driver"
[Severity: High] Is it possible this Kconfig entry is missing a depends on HID? If the HID subsystem is disabled or configured as a module while this driver is built-in, could this result in linking errors for HID core functions like hid_register_driver() and hid_parse()?
+ help + Say Y to enable support of the tablet mode switch driver for
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/misc/amd_sfh_tabletmode.c b/drivers/input/misc/amd_sfh_tabletmode.c new file mode 100644 index 0000000000000..ee1207372207c --- /dev/null +++ b/drivers/input/misc/amd_sfh_tabletmode.c
[ ... ]
+static int amd_sfh_tabletmode_event(struct hid_device *hid, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ input_report_switch(field->hidinput->input, SW_TABLET_MODE, value);[Severity: High] Does this unconditionally dereference field->hidinput, which could be NULL for unmapped usages? Because the driver struct below lacks a .usage_table, hid_match_usage() will default to true for all usages, meaning this callback intercepts every incoming HID report field. This could lead to a NULL pointer dereference, and will also blindly emit a SW_TABLET_MODE event for any report, causing standard buttons or battery reports to randomly toggle tablet mode. Since .input_mapping already maps the usage to EV_SW, allowing the generic HID core to automatically report the event, could this callback be removed entirely?
+
+ return 0;
+}
+
+static const struct hid_device_id amd_sfh_tabletmode_devices[] = {
+ { HID_DEVICE(BUS_AMD_SFH, HID_GROUP_GENERIC, AMD_SFH_HID_VENDOR, AMD_SFH_HID_PRODUCT) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, amd_sfh_tabletmode_devices);
+
+static struct hid_driver amd_sfh_tabletmode_driver = {
+ .name = "amd_sfh_tabletmode",
+ .id_table = amd_sfh_tabletmode_devices,
+ .probe = amd_sfh_tabletmode_probe,
+ .remove = amd_sfh_tabletmode_remove,
+ .input_mapping = amd_sfh_tabletmode_input_mapping,
+ .event = amd_sfh_tabletmode_event,[Severity: High] Should this struct include a .usage_table? Without it, all device usages are routed to the .event callback above.
+};
-- Sashiko AI review · https://sashiko.dev/#/patchset/ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com?part=2