| 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: <c2abcc55-8f85-4f46-8d9d-4cb74782942a@linux.intel.com>
Date: Tue, 9 Dec 2025 11:14:05 +0200 (EET)
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 <hansg@...nel.org>,
Denis Benato <benato.denis96@...il.com>
Subject: Re: [PATCH v10 05/11] HID: asus: move vendor initialization to
probe
On Tue, 9 Dec 2025, Antheas Kapenekakis wrote:
> On Mon, 8 Dec 2025 at 18:11, Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
> >
> > On Sat, 22 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > ROG NKEY devices have multiple HID endpoints, around 3-4. One of those
> > > endpoints has a usage page of 0xff31, and is the one that emits keyboard
> > > shortcuts and controls RGB/backlight. Currently, this driver places
> > > the usage page check under asus_input_mapping and then inits backlight
> > > in asus_input_configured which is unnecessarily complicated and prevents
> > > probe from performing customizations on the vendor endpoint.
> > >
> > > Simplify the logic by introducing an is_vendor variable into probe that
> > > checks for usage page 0xff31. Then, use this variable to move backlight
> > > initialization into probe instead of asus_input_configured, and remove
> > > the backlight check from asus_input_mapping.
> >
> > In the changelogs, please add () after any name that refers to a C
> > function so the reader immediately knows you're talking about a function.
>
> Sure. What should be my timeline for fixing this?
>
> I assume we missed the merge window. If not, I can tweak the series
> and send it later today
This is 6.20 material so no big hurry.
--
i.
> > > Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> > > ---
> > > drivers/hid/hid-asus.c | 35 ++++++++++++++++++-----------------
> > > 1 file changed, 18 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > index 2a412e10f916..faac971794c0 100644
> > > --- a/drivers/hid/hid-asus.c
> > > +++ b/drivers/hid/hid-asus.c
> > > @@ -48,6 +48,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
> > > @@ -127,7 +128,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;
> > > @@ -318,7 +318,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",
> > > @@ -941,11 +941,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;
> > > }
> > >
> > > @@ -1018,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;
> > > }
> > > @@ -1143,8 +1129,11 @@ 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;
> > > + bool is_vendor = false;
> > > + int ret;
> > >
> > > drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
> > > if (drvdata == NULL) {
> > > @@ -1228,12 +1217,24 @@ 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;
> > > + }
> > > +
> > > 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
> > >
> >
> > --
> > i.
> >
> >
>
Powered by blists - more mailing lists