| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c5b8c060-9fab-acda-b002-cf57716265ed@linux.intel.com>
Date: Wed, 15 Oct 2025 13:59:43 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Antheas Kapenekakis <lkml@...heas.dev>
cc: platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>,
Corentin Chary <corentin.chary@...il.com>,
"Luke D . Jones" <luke@...nes.dev>, Hans de Goede <hdegoede@...hat.com>,
Denis Benato <benato.denis96@...il.com>
Subject: Re: [PATCH v6 2/7] HID: asus: prevent binding to all HID devices on
ROG
On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> ROG keyboards are HID compliant and only care about the endpoint that
> produces vendor events (e.g., fan mode) and has the keyboard backlight.
>
> Therefore, handle all of the endpoints of ROG keyboards as compliant,
> by adding HID_QUIRK_INPUT_PER_APP and, for devices other than the vendor
> one, by adding QUIRK_HANDLE_GENERIC to stop mutating them.
>
> Due to HID_QUIRK_INPUT_PER_APP, rgb register is moved into probe, as
> the input_* functions are called multiple times (4 for the Z13).
>
> Reviewed-by: Luke D. Jones <luke@...nes.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
> drivers/hid/hid-asus.c | 71 +++++++++++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d0c783df99bc..a62559e3e064 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -47,6 +47,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define T100CHI_MOUSE_REPORT_ID 0x06
> #define FEATURE_REPORT_ID 0x0d
> #define INPUT_REPORT_ID 0x5d
> +#define HID_USAGE_PAGE_VENDOR 0xff310000
> #define FEATURE_KBD_REPORT_ID 0x5a
> #define FEATURE_KBD_REPORT_SIZE 64
> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> @@ -89,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> +#define QUIRK_HANDLE_GENERIC BIT(14)
>
> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> QUIRK_NO_INIT_REPORTS | \
> @@ -125,7 +127,6 @@ struct asus_drvdata {
> struct input_dev *tp_kbd_input;
> struct asus_kbd_leds *kbd_backlight;
> const struct asus_touchpad_info *tp;
> - bool enable_backlight;
> struct power_supply *battery;
> struct power_supply_desc battery_desc;
> int battery_capacity;
> @@ -316,7 +317,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
> static int asus_event(struct hid_device *hdev, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> - if ((usage->hid & HID_USAGE_PAGE) == 0xff310000 &&
> + if ((usage->hid & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR &&
> (usage->hid & HID_USAGE) != 0x00 &&
> (usage->hid & HID_USAGE) != 0xff && !usage->type) {
> hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
> @@ -331,6 +332,10 @@ static int asus_raw_event(struct hid_device *hdev,
> {
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>
> + /* NOOP on generic HID devices to avoid side effects. */
> + if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
> + return 0;
> +
> if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
> return asus_report_battery(drvdata, data, size);
>
> @@ -875,6 +880,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> struct input_dev *input = hi->input;
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>
> + /* NOOP on generic HID devices to avoid side effects. */
> + if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
> + return 0;
> +
> /* T100CHI uses MULTI_INPUT, bind the touchpad to the mouse hid_input */
> if (drvdata->quirks & QUIRK_T100CHI &&
> hi->report->id != T100CHI_MOUSE_REPORT_ID)
> @@ -928,11 +937,6 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>
> drvdata->input = input;
>
> - if (drvdata->enable_backlight &&
> - !asus_kbd_wmi_led_control_present(hdev) &&
> - asus_kbd_register_leds(hdev))
> - hid_warn(hdev, "Failed to initialize backlight.\n");
> -
> return 0;
> }
>
> @@ -952,6 +956,10 @@ static int asus_input_mapping(struct hid_device *hdev,
> return -1;
> }
>
> + /* NOOP on generic HID devices to avoid side effects. */
> + if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
> + return 0;
> +
> /*
> * Ignore a bunch of bogus collections in the T100CHI descriptor.
> * This avoids a bunch of non-functional hid_input devices getting
> @@ -1005,15 +1013,6 @@ static int asus_input_mapping(struct hid_device *hdev,
> return -1;
> }
>
> - /*
> - * Check and enable backlight only on devices with UsagePage ==
> - * 0xff31 to avoid initializing the keyboard firmware multiple
> - * times on devices with multiple HID descriptors but same
> - * PID/VID.
> - */
> - if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
> - drvdata->enable_backlight = true;
> -
> set_bit(EV_REP, hi->input->evbit);
> return 1;
> }
> @@ -1130,8 +1129,10 @@ static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
>
> static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> - int ret;
> + struct hid_report_enum *rep_enum;
> struct asus_drvdata *drvdata;
> + struct hid_report *rep;
> + int ret, is_vendor = 0;
>
> drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
> if (drvdata == NULL) {
> @@ -1215,18 +1216,42 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> + /* Check for vendor for RGB init and handle generic devices properly. */
> + rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
> + list_for_each_entry(rep, &rep_enum->report_list, list) {
> + if ((rep->application & HID_USAGE_PAGE) == HID_USAGE_PAGE_VENDOR)
> + is_vendor = true;
> + }
> +
> + /*
> + * For ROG keyboards, make them hid compliant by
hid -> HID
> + * creating one input per application. For interfaces other than
> + * the vendor one, disable hid-asus handlers.
Please reflow to 80 chars.
> + */
> + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> + if (!is_vendor)
> + drvdata->quirks |= QUIRK_HANDLE_GENERIC;
> + hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
> + }
> +
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret) {
> hid_err(hdev, "Asus hw start failed: %d\n", ret);
> return ret;
> }
>
> + if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> + !asus_kbd_wmi_led_control_present(hdev) &&
> + asus_kbd_register_leds(hdev))
> + hid_warn(hdev, "Failed to initialize backlight.\n");
> +
> /*
> - * Check that input registration succeeded. Checking that
> - * HID_CLAIMED_INPUT is set prevents a UAF when all input devices
> - * were freed during registration due to no usages being mapped,
> - * leaving drvdata->input pointing to freed memory.
> + * For ROG keyboards, skip rename for consistency and ->input check as
> + * some devices do not have inputs.
> */
> + if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD)
> + return 0;
> +
> if (!drvdata->input || !(hdev->claimed & HID_CLAIMED_INPUT)) {
> hid_err(hdev, "Asus input not registered\n");
> ret = -ENOMEM;
> @@ -1277,6 +1302,10 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> {
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>
> + /* NOOP on generic HID devices to avoid side effects. */
> + if (drvdata->quirks & QUIRK_HANDLE_GENERIC)
> + return rdesc;
> +
> if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
> *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) {
> hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
>
--
i.
Powered by blists - more mailing lists