* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 12:14 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <160c3adf-9333-4486-ba4c-d3359ea73337@gmail.com>
On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/13/25 22:15, Antheas Kapenekakis wrote:
> > This is a two part series which does the following:
> > - Clean-up init sequence
> > - Unify backlight handling to happen under asus-wmi so that all Aura
> > devices have synced brightness controls and the backlight button works
> > properly when it is on a USB laptop keyboard instead of one w/ WMI.
> >
> > For more context, see cover letter of V1. Since V5, I removed some patches
> > to make this easier to merge.
> >
> > All comments with these patches had been addressed since V4.
> I have loaded this patchset for users of asus-linux project to try out.
>
> One of them opened a bug report about a kernel bug that happens
> consistently when closing the lid of his laptop [1].
>
> He also sent another piece of kernel log, but didn't specify anything more
> about this [2].
>
> [1] https://pastebin.com/akZx1w10
> [2] https://pastebin.com/sKdczPgf
Can you provide a link to the bug report? [2] seems unrelated.
As for [1], it looks like a trace that stems from a sysfs write to
brightness stemming from userspace that follows the same chain it
would on a stock kernel and times out. Is it present on a stock
kernel?
Ilpo should know more about this, could the spinlock be interfering?
My testing on devices that have WMI led controls is a bit limited
unfortunately. However, most of our asus users have been happy with
this series for around half a year now.
Antheas
> > ---
> > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> >
> > Changes since V5:
> > - It's been a long time
> > - Remove addition of RGB as that had some comments I need to work on
> > - Remove folio patch (already merged)
> > - Remove legacy fix patch 11 from V4. There is a small chance that
> > without this patch, some old NKEY keyboards might not respond to
> > RGB commands according to Luke, but the kernel driver does not do
> > RGB currently. The 0x5d init is done by Armoury crate software in
> > Windows. If an issue is found, we can re-add it or just remove patches
> > 1/2 before merging. However, init could use the cleanup.
> >
> > Changes since V4:
> > - Fix KConfig (reported by kernel robot)
> > - Fix Ilpo's nits, if I missed anything lmk
> >
> > Changes since V3:
> > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > that it is not needed for their media keys to function.
> > - Cover init in asus-wmi with spinlock as per Hans
> > - If asus-wmi registers WMI handler with brightness, init the brightness
> > in USB Asus keyboards, per Hans.
> > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > the spin lock (but after the asus reference is set to null)
> >
> > Changes since V2:
> > - Check lazy init succeds in asus-wmi before setting register variable
> > - make explicit check in asus_hid_register_listener for listener existing
> > to avoid re-init
> > - rename asus_brt to asus_hid in most places and harmonize everything
> > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > all input vendor devices. This includes moving rgb init to probe
> > instead of the input_configured callbacks.
> > - Remove fan key (during retest it appears to be 0xae that is already
> > supported by hid-asus)
> > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > - removes fds from userspace and breaks backlight functionality. All
> > - current mainline drivers do not support backlight hotplugging, so most
> > userspace software (e.g., KDE, UPower) is built with that assumption.
> > For the Ally, since it disconnects its controller during sleep, this
> > caused the backlight slider to not work in KDE.
> >
> > Changes since V1:
> > - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > - Fix ifdef else having an invalid signature (reported by kernel robot)
> > - Restore input arguments to init and keyboard function so they can
> > be re-used for RGB controls.
> > - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > keyboard rename into it.
> > - Unregister brightness listener before removing work queue to avoid
> > a race condition causing corruption
> > - Remove spurious mutex unlock in asus_brt_event
> > - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > relocking the mutex and causing a deadlock when unregistering leds
> > - Add extra check during unregistering to avoid calling unregister when
> > no led device is registered.
> > - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > the driver to create 4 RGB handlers per device. I also suspect some
> > extra events sneak through (KDE had the @@@@@@).
> >
> > Antheas Kapenekakis (7):
> > HID: asus: refactor init sequence per spec
> > HID: asus: prevent binding to all HID devices on ROG
> > platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > HID: asus: listen to the asus-wmi brightness device instead of
> > creating one
> > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > platform/x86: asus-wmi: add keyboard brightness event handler
> > HID: asus: add support for the asus-wmi brightness handler
> >
> > drivers/hid/hid-asus.c | 235 +++++++++++----------
> > drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > 3 files changed, 291 insertions(+), 170 deletions(-)
> >
> >
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-16 11:57 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251013201535.6737-1-lkml@antheas.dev>
On 10/13/25 22:15, Antheas Kapenekakis wrote:
> This is a two part series which does the following:
> - Clean-up init sequence
> - Unify backlight handling to happen under asus-wmi so that all Aura
> devices have synced brightness controls and the backlight button works
> properly when it is on a USB laptop keyboard instead of one w/ WMI.
>
> For more context, see cover letter of V1. Since V5, I removed some patches
> to make this easier to merge.
>
> All comments with these patches had been addressed since V4.
I have loaded this patchset for users of asus-linux project to try out.
One of them opened a bug report about a kernel bug that happens
consistently when closing the lid of his laptop [1].
He also sent another piece of kernel log, but didn't specify anything more
about this [2].
[1] https://pastebin.com/akZx1w10
[2] https://pastebin.com/sKdczPgf
> ---
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V5:
> - It's been a long time
> - Remove addition of RGB as that had some comments I need to work on
> - Remove folio patch (already merged)
> - Remove legacy fix patch 11 from V4. There is a small chance that
> without this patch, some old NKEY keyboards might not respond to
> RGB commands according to Luke, but the kernel driver does not do
> RGB currently. The 0x5d init is done by Armoury crate software in
> Windows. If an issue is found, we can re-add it or just remove patches
> 1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
> - Fix KConfig (reported by kernel robot)
> - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
> - Add initializer for 0x5d for old NKEY keyboards until it is verified
> that it is not needed for their media keys to function.
> - Cover init in asus-wmi with spinlock as per Hans
> - If asus-wmi registers WMI handler with brightness, init the brightness
> in USB Asus keyboards, per Hans.
> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> - Fix oops when unregistering asus-wmi by moving unregister outside of
> the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
> - Check lazy init succeds in asus-wmi before setting register variable
> - make explicit check in asus_hid_register_listener for listener existing
> to avoid re-init
> - rename asus_brt to asus_hid in most places and harmonize everything
> - switch to a spinlock instead of a mutex to avoid kernel ooops
> - fixup hid device quirks to avoid multiple RGB devices while still exposing
> all input vendor devices. This includes moving rgb init to probe
> instead of the input_configured callbacks.
> - Remove fan key (during retest it appears to be 0xae that is already
> supported by hid-asus)
> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> - removes fds from userspace and breaks backlight functionality. All
> - current mainline drivers do not support backlight hotplugging, so most
> userspace software (e.g., KDE, UPower) is built with that assumption.
> For the Ally, since it disconnects its controller during sleep, this
> caused the backlight slider to not work in KDE.
>
> Changes since V1:
> - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> - Fix ifdef else having an invalid signature (reported by kernel robot)
> - Restore input arguments to init and keyboard function so they can
> be re-used for RGB controls.
> - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> keyboard rename into it.
> - Unregister brightness listener before removing work queue to avoid
> a race condition causing corruption
> - Remove spurious mutex unlock in asus_brt_event
> - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> relocking the mutex and causing a deadlock when unregistering leds
> - Add extra check during unregistering to avoid calling unregister when
> no led device is registered.
> - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> the driver to create 4 RGB handlers per device. I also suspect some
> extra events sneak through (KDE had the @@@@@@).
>
> Antheas Kapenekakis (7):
> HID: asus: refactor init sequence per spec
> HID: asus: prevent binding to all HID devices on ROG
> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> HID: asus: listen to the asus-wmi brightness device instead of
> creating one
> platform/x86: asus-wmi: remove unused keyboard backlight quirk
> platform/x86: asus-wmi: add keyboard brightness event handler
> HID: asus: add support for the asus-wmi brightness handler
>
> drivers/hid/hid-asus.c | 235 +++++++++++----------
> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> 3 files changed, 291 insertions(+), 170 deletions(-)
>
>
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
^ permalink raw reply
* Re: [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Ilpo Järvinen @ 2025-10-16 10:38 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <CAGwozwEo+Mx8fv_ohQQ9Ha6p=bJcJmfj==aRGcgoqQXzXEZmVw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4253 bytes --]
On Thu, 16 Oct 2025, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 12:19, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
> >
> > > On Wed, 15 Oct 2025 at 13:59, Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > >
> > > > On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> > > >
> > > > > Some devices, such as the Z13 have multiple AURA devices connected
> > > > > to them by USB. In addition, they might have a WMI interface for
> > > > > RGB. In Windows, Armoury Crate exposes a unified brightness slider
> > > > > for all of them, with 3 brightness levels.
> > > > >
> > > > > Therefore, to be synergistic in Linux, and support existing tooling
> > > > > such as UPower, allow adding listeners to the RGB device of the WMI
> > > > > interface. If WMI does not exist, lazy initialize the interface.
> > > > >
> > > > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > > > ---
> > > > > drivers/platform/x86/asus-wmi.c | 118 ++++++++++++++++++---
> > > > > include/linux/platform_data/x86/asus-wmi.h | 16 +++
> > > > > 2 files changed, 121 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > > index e72a2b5d158e..a2a7cd61fd59 100644
> > > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > > +++ b/drivers/platform/x86/asus-wmi.c
> > > > > @@ -258,6 +258,8 @@ struct asus_wmi {
> > > > > int tpd_led_wk;
> > > > > struct led_classdev kbd_led;
> > > > > int kbd_led_wk;
> > > > > + bool kbd_led_avail;
> > > > > + bool kbd_led_registered;
> > > > > struct led_classdev lightbar_led;
> > > > > int lightbar_led_wk;
> > > > > struct led_classdev micmute_led;
> > > > > @@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
> > > > >
> > > > > /* LEDs ***********************************************************************/
> > > > >
> > > > > +struct asus_hid_ref {
> > > > > + struct list_head listeners;
> > > > > + struct asus_wmi *asus;
> > > > > + spinlock_t lock;
> > > >
> > > > Please always document what a lock protects.
> > > >
> > > > I started wonder why it needs to be spinlock?
> > > >
> > > > It would seem rwsem is more natural for it as write is only needed at
> > > > probe/remove time (if there's no good reason for using a spinlock).
> > > >
> > > > You're also missing include.
> > >
> > > I went through the comments. Thanks. The reason that it is a spinlock
> > > is that both hid-asus and asus-wmi interact with the primitives to
> > > register and unregister listeners, either of which can prompt the
> > > creation of the led device which has to be atomic. And they do so from
> > > IRQs too.
> >
> > Please note in the changelog how it can happen from IRQs as I tried but
> > couldn't find anything. Admittedly, I didn't try to follow the callchains
> > that deeply. The justification should be clear enough to anyone who
> > looks this commit later so better have it in the changelog.
>
> The initial versions used a mutex, and we found kernel hangs under
> IRQs, so it was converted to a spinlock. I will try to reword.
> Specifically, I think the errors came when holding the lock when
> changing brightness.
>
> I recall one of them was brightness hotkey (second to last patch)
> triggers an IRQ -> event goes to asus-wmi -> calls brightness handler
> -> tries to hold lock -> crashes. Lock needs to be held because
> hid-asus can choose to unregister a listener.
If you're not certain and want an easy way to obtain a stacktrace to
confirm, you can likely do something like:
WARN_ON_ONCE(in_atomic());
...at where you are taking the lock.
--
i.
> > > Perhaps the driver could be refactored to use rwsem, I am not sure.
> >
> > Just leave it spinlock.
> >
> > > The fixed version can be found here[1]. I will give it 1-2 more days
> > > in case someone else wants to chime in and resend.
> >
> >
> >
> > --
> > i.
>
^ permalink raw reply
* Re: [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Antheas Kapenekakis @ 2025-10-16 10:23 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <cf824f48-58b4-2400-9acf-796bb76d6b28@linux.intel.com>
On Thu, 16 Oct 2025 at 12:19, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
>
> > On Wed, 15 Oct 2025 at 13:59, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> > >
> > > > Some devices, such as the Z13 have multiple AURA devices connected
> > > > to them by USB. In addition, they might have a WMI interface for
> > > > RGB. In Windows, Armoury Crate exposes a unified brightness slider
> > > > for all of them, with 3 brightness levels.
> > > >
> > > > Therefore, to be synergistic in Linux, and support existing tooling
> > > > such as UPower, allow adding listeners to the RGB device of the WMI
> > > > interface. If WMI does not exist, lazy initialize the interface.
> > > >
> > > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > > ---
> > > > drivers/platform/x86/asus-wmi.c | 118 ++++++++++++++++++---
> > > > include/linux/platform_data/x86/asus-wmi.h | 16 +++
> > > > 2 files changed, 121 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > > index e72a2b5d158e..a2a7cd61fd59 100644
> > > > --- a/drivers/platform/x86/asus-wmi.c
> > > > +++ b/drivers/platform/x86/asus-wmi.c
> > > > @@ -258,6 +258,8 @@ struct asus_wmi {
> > > > int tpd_led_wk;
> > > > struct led_classdev kbd_led;
> > > > int kbd_led_wk;
> > > > + bool kbd_led_avail;
> > > > + bool kbd_led_registered;
> > > > struct led_classdev lightbar_led;
> > > > int lightbar_led_wk;
> > > > struct led_classdev micmute_led;
> > > > @@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
> > > >
> > > > /* LEDs ***********************************************************************/
> > > >
> > > > +struct asus_hid_ref {
> > > > + struct list_head listeners;
> > > > + struct asus_wmi *asus;
> > > > + spinlock_t lock;
> > >
> > > Please always document what a lock protects.
> > >
> > > I started wonder why it needs to be spinlock?
> > >
> > > It would seem rwsem is more natural for it as write is only needed at
> > > probe/remove time (if there's no good reason for using a spinlock).
> > >
> > > You're also missing include.
> >
> > I went through the comments. Thanks. The reason that it is a spinlock
> > is that both hid-asus and asus-wmi interact with the primitives to
> > register and unregister listeners, either of which can prompt the
> > creation of the led device which has to be atomic. And they do so from
> > IRQs too.
>
> Please note in the changelog how it can happen from IRQs as I tried but
> couldn't find anything. Admittedly, I didn't try to follow the callchains
> that deeply. The justification should be clear enough to anyone who
> looks this commit later so better have it in the changelog.
The initial versions used a mutex, and we found kernel hangs under
IRQs, so it was converted to a spinlock. I will try to reword.
Specifically, I think the errors came when holding the lock when
changing brightness.
I recall one of them was brightness hotkey (second to last patch)
triggers an IRQ -> event goes to asus-wmi -> calls brightness handler
-> tries to hold lock -> crashes. Lock needs to be held because
hid-asus can choose to unregister a listener.
> > Perhaps the driver could be refactored to use rwsem, I am not sure.
>
> Just leave it spinlock.
>
> > The fixed version can be found here[1]. I will give it 1-2 more days
> > in case someone else wants to chime in and resend.
>
>
>
> --
> i.
^ permalink raw reply
* Re: [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Ilpo Järvinen @ 2025-10-16 10:18 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <CAGwozwFBQ4DWS5s-La5f-6H=ZQvQFjU3=7U2RiJStGxO1sM+bQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]
On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
> On Wed, 15 Oct 2025 at 13:59, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> >
> > > Some devices, such as the Z13 have multiple AURA devices connected
> > > to them by USB. In addition, they might have a WMI interface for
> > > RGB. In Windows, Armoury Crate exposes a unified brightness slider
> > > for all of them, with 3 brightness levels.
> > >
> > > Therefore, to be synergistic in Linux, and support existing tooling
> > > such as UPower, allow adding listeners to the RGB device of the WMI
> > > interface. If WMI does not exist, lazy initialize the interface.
> > >
> > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > Tested-by: Luke D. Jones <luke@ljones.dev>
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > > drivers/platform/x86/asus-wmi.c | 118 ++++++++++++++++++---
> > > include/linux/platform_data/x86/asus-wmi.h | 16 +++
> > > 2 files changed, 121 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > index e72a2b5d158e..a2a7cd61fd59 100644
> > > --- a/drivers/platform/x86/asus-wmi.c
> > > +++ b/drivers/platform/x86/asus-wmi.c
> > > @@ -258,6 +258,8 @@ struct asus_wmi {
> > > int tpd_led_wk;
> > > struct led_classdev kbd_led;
> > > int kbd_led_wk;
> > > + bool kbd_led_avail;
> > > + bool kbd_led_registered;
> > > struct led_classdev lightbar_led;
> > > int lightbar_led_wk;
> > > struct led_classdev micmute_led;
> > > @@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
> > >
> > > /* LEDs ***********************************************************************/
> > >
> > > +struct asus_hid_ref {
> > > + struct list_head listeners;
> > > + struct asus_wmi *asus;
> > > + spinlock_t lock;
> >
> > Please always document what a lock protects.
> >
> > I started wonder why it needs to be spinlock?
> >
> > It would seem rwsem is more natural for it as write is only needed at
> > probe/remove time (if there's no good reason for using a spinlock).
> >
> > You're also missing include.
>
> I went through the comments. Thanks. The reason that it is a spinlock
> is that both hid-asus and asus-wmi interact with the primitives to
> register and unregister listeners, either of which can prompt the
> creation of the led device which has to be atomic. And they do so from
> IRQs too.
Please note in the changelog how it can happen from IRQs as I tried but
couldn't find anything. Admittedly, I didn't try to follow the callchains
that deeply. The justification should be clear enough to anyone who
looks this commit later so better have it in the changelog.
> Perhaps the driver could be refactored to use rwsem, I am not sure.
Just leave it spinlock.
> The fixed version can be found here[1]. I will give it 1-2 more days
> in case someone else wants to chime in and resend.
--
i.
^ permalink raw reply
* [syzbot] Monthly input report (Oct 2025)
From: syzbot @ 2025-10-16 8:36 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 2 new issues were detected and 0 were fixed.
In total, 21 issues are still open and 62 have already been fixed.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 2969 Yes WARNING in cm109_urb_irq_callback/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<2> 1436 No possible deadlock in evdev_pass_values (2)
https://syzkaller.appspot.com/bug?extid=13d3cb2a3dc61e6092f5
<3> 964 Yes WARNING in enable_work
https://syzkaller.appspot.com/bug?extid=7053fbd8757fecbbe492
<4> 99 Yes WARNING in cm109_input_open/usb_submit_urb (3)
https://syzkaller.appspot.com/bug?extid=ac0f9c4cc1e034160492
<5> 64 Yes possible deadlock in uinput_request_submit
https://syzkaller.appspot.com/bug?extid=159077b1355b8cd72757
<6> 55 Yes INFO: rcu detected stall in sys_pselect6 (2)
https://syzkaller.appspot.com/bug?extid=310c88228172bcf54bef
<7> 32 No KASAN: slab-use-after-free Read in report_descriptor_read
https://syzkaller.appspot.com/bug?extid=bc537ca7a0efe33988eb
<8> 10 Yes INFO: task hung in console_callback (6)
https://syzkaller.appspot.com/bug?extid=6027421afa74a2ba440d
<9> 3 Yes possible deadlock in input_ff_flush
https://syzkaller.appspot.com/bug?extid=ed7c6209f62eba1565aa
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
From: Jiri Kosina @ 2025-10-16 8:27 UTC (permalink / raw)
To: Zhang, Lixu
Cc: srinivas pandruvada, linux-input@vger.kernel.org,
benjamin.tissoires@redhat.com, Wang, Selina
In-Reply-To: <SJ0PR11MB561382CBA96A7591BABC6FB093E9A@SJ0PR11MB5613.namprd11.prod.outlook.com>
On Thu, 16 Oct 2025, Zhang, Lixu wrote:
> >> I personally prefer to have this patch merged before the patch series
> >> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a
> >> merge conflict exists between this patch and that patch series. If
> >> this patch is merged first, it would be more convenient for others to
> >> backport this patch to other kernel versions. BTW, in this case, the
> >> patch series [PATCH 0/6] HID: intel-ish-hid: Various power management
> >> would need to be rebased. Do I need to send a v2 patch series for the
> >rebase?
> >
> >We can definitely do that. The question is whether this one should go in right
> >now for 6.18 still, or we queue both for 6.19.
> You can queue both for 6.19. Thanks.
Could you then please send me the power management series rebased on top
of it, and I'll create a -v2 topic branch with all of it together.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
From: Zhang, Lixu @ 2025-10-16 0:42 UTC (permalink / raw)
To: Jiri Kosina
Cc: srinivas pandruvada, linux-input@vger.kernel.org,
benjamin.tissoires@redhat.com, Wang, Selina
In-Reply-To: <8nr40316-26r2-6893-qqpr-rq37p2nsn7r2@xreary.bet>
>-----Original Message-----
>From: Jiri Kosina <jikos@kernel.org>
>Sent: Wednesday, October 15, 2025 10:08 PM
>To: Zhang, Lixu <lixu.zhang@intel.com>
>Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; linux-
>input@vger.kernel.org; benjamin.tissoires@redhat.com; Wang, Selina
><selina.wang@intel.com>
>Subject: RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to
>prevent resume blocking
>
>On Wed, 15 Oct 2025, Zhang, Lixu wrote:
>
>> I personally prefer to have this patch merged before the patch series
>> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a
>> merge conflict exists between this patch and that patch series. If
>> this patch is merged first, it would be more convenient for others to
>> backport this patch to other kernel versions. BTW, in this case, the
>> patch series [PATCH 0/6] HID: intel-ish-hid: Various power management
>> would need to be rebased. Do I need to send a v2 patch series for the
>rebase?
>
>We can definitely do that. The question is whether this one should go in right
>now for 6.18 still, or we queue both for 6.19.
You can queue both for 6.19. Thanks.
Best regards,
Lixu
>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs
^ permalink raw reply
* Re: [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Antheas Kapenekakis @ 2025-10-15 15:45 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <cf0ca840-6e0d-2d99-cb23-eabf0ac5263b@linux.intel.com>
On Wed, 15 Oct 2025 at 13:59, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
>
> > Some devices, such as the Z13 have multiple AURA devices connected
> > to them by USB. In addition, they might have a WMI interface for
> > RGB. In Windows, Armoury Crate exposes a unified brightness slider
> > for all of them, with 3 brightness levels.
> >
> > Therefore, to be synergistic in Linux, and support existing tooling
> > such as UPower, allow adding listeners to the RGB device of the WMI
> > interface. If WMI does not exist, lazy initialize the interface.
> >
> > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > Tested-by: Luke D. Jones <luke@ljones.dev>
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> > drivers/platform/x86/asus-wmi.c | 118 ++++++++++++++++++---
> > include/linux/platform_data/x86/asus-wmi.h | 16 +++
> > 2 files changed, 121 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index e72a2b5d158e..a2a7cd61fd59 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -258,6 +258,8 @@ struct asus_wmi {
> > int tpd_led_wk;
> > struct led_classdev kbd_led;
> > int kbd_led_wk;
> > + bool kbd_led_avail;
> > + bool kbd_led_registered;
> > struct led_classdev lightbar_led;
> > int lightbar_led_wk;
> > struct led_classdev micmute_led;
> > @@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
> >
> > /* LEDs ***********************************************************************/
> >
> > +struct asus_hid_ref {
> > + struct list_head listeners;
> > + struct asus_wmi *asus;
> > + spinlock_t lock;
>
> Please always document what a lock protects.
>
> I started wonder why it needs to be spinlock?
>
> It would seem rwsem is more natural for it as write is only needed at
> probe/remove time (if there's no good reason for using a spinlock).
>
> You're also missing include.
I went through the comments. Thanks. The reason that it is a spinlock
is that both hid-asus and asus-wmi interact with the primitives to
register and unregister listeners, either of which can prompt the
creation of the led device which has to be atomic. And they do so from
IRQs too.
Perhaps the driver could be refactored to use rwsem, I am not sure.
The fixed version can be found here[1]. I will give it 1-2 more days
in case someone else wants to chime in and resend.
Antheas
[1] https://github.com/bazzite-org/patchwork/commits/asusrgb/
> > +};
> > +
> > +struct asus_hid_ref asus_ref = {
>
> Should be static ?
>
> > + .listeners = LIST_HEAD_INIT(asus_ref.listeners),
> > + .asus = NULL,
> > + .lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
> > +};
> > +
> > +int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > +{
> > + unsigned long flags;
> > + int ret = 0;
> > +
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + list_add_tail(&bdev->list, &asus_ref.listeners);
> > + if (asus_ref.asus) {
> > + if (asus_ref.asus->kbd_led_registered && asus_ref.asus->kbd_led_wk >= 0)
> > + bdev->brightness_set(bdev, asus_ref.asus->kbd_led_wk);
> > +
> > + if (!asus_ref.asus->kbd_led_registered) {
> > + ret = led_classdev_register(
> > + &asus_ref.asus->platform_device->dev,
> > + &asus_ref.asus->kbd_led);
> > + if (!ret)
> > + asus_ref.asus->kbd_led_registered = true;
>
> I suggest you use guard() for the spinlock and return early where possible.
>
> With guard() in use, you can do normal error handling here with return ret
> immediately.
>
> Please also add a local var for asus_ref.asus.
>
> > + }
> > + }
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_hid_register_listener);
> > +
> > +void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + list_del(&bdev->list);
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > +}
> > +EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
> > +
> > /*
> > * These functions actually update the LED's, and are called from a
> > * workqueue. By doing this as separate work rather than when the LED
> > @@ -1609,6 +1658,7 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
> >
> > static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > {
> > + struct asus_hid_listener *listener;
> > struct asus_wmi *asus;
> > int max_level;
> >
> > @@ -1616,25 +1666,39 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > max_level = asus->kbd_led.max_brightness;
> >
> > asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > - kbd_led_update(asus);
> > +
> > + if (asus->kbd_led_avail)
> > + kbd_led_update(asus);
> > +
> > + list_for_each_entry(listener, &asus_ref.listeners, list)
> > + listener->brightness_set(listener, asus->kbd_led_wk);
> > }
> >
> > static void kbd_led_set(struct led_classdev *led_cdev,
> > enum led_brightness value)
> > {
> > + unsigned long flags;
> > +
> > /* Prevent disabling keyboard backlight on module unregister */
> > if (led_cdev->flags & LED_UNREGISTERING)
> > return;
> >
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > do_kbd_led_set(led_cdev, value);
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > }
> >
> > static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
> > {
> > - struct led_classdev *led_cdev = &asus->kbd_led;
> > + struct led_classdev *led_cdev;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + led_cdev = &asus->kbd_led;
> >
>
> I'd remove the empty line from the critical section.
>
> I think you should mention the locking in the changelog too as it clearly
> impacts not only the new code.
>
> > do_kbd_led_set(led_cdev, value);
> > led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > }
> >
> > static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
> > @@ -1644,6 +1708,9 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
> >
> > asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> >
> > + if (!asus->kbd_led_avail)
> > + return asus->kbd_led_wk;
> > +
> > retval = kbd_led_read(asus, &value, NULL);
> > if (retval < 0)
> > return retval;
> > @@ -1759,7 +1826,15 @@ static int camera_led_set(struct led_classdev *led_cdev,
> >
> > static void asus_wmi_led_exit(struct asus_wmi *asus)
> > {
> > - led_classdev_unregister(&asus->kbd_led);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + asus_ref.asus = NULL;
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > +
> > + if (asus->kbd_led_registered)
> > + led_classdev_unregister(&asus->kbd_led);
> > +
> > led_classdev_unregister(&asus->tpd_led);
> > led_classdev_unregister(&asus->wlan_led);
> > led_classdev_unregister(&asus->lightbar_led);
> > @@ -1773,6 +1848,8 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
> > static int asus_wmi_led_init(struct asus_wmi *asus)
> > {
> > int rv = 0, num_rgb_groups = 0, led_val;
> > + struct asus_hid_listener *listener;
> > + unsigned long flags;
> >
> > if (asus->kbd_rgb_dev)
> > kbd_rgb_mode_groups[num_rgb_groups++] = &kbd_rgb_mode_group;
> > @@ -1797,23 +1874,38 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > goto error;
> > }
> >
> > - if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
> > - pr_info("using asus-wmi for asus::kbd_backlight\n");
> > + asus->kbd_led.name = "asus::kbd_backlight";
> > + asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > + asus->kbd_led.brightness_set = kbd_led_set;
> > + asus->kbd_led.brightness_get = kbd_led_get;
> > + asus->kbd_led.max_brightness = 3;
> > + asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > +
> > + if (asus->kbd_led_avail)
> > asus->kbd_led_wk = led_val;
> > - asus->kbd_led.name = "asus::kbd_backlight";
> > - asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > - asus->kbd_led.brightness_set = kbd_led_set;
> > - asus->kbd_led.brightness_get = kbd_led_get;
> > - asus->kbd_led.max_brightness = 3;
> > + else
> > + asus->kbd_led_wk = -1;
> >
> > - if (num_rgb_groups != 0)
> > - asus->kbd_led.groups = kbd_rgb_mode_groups;
> > + if (asus->kbd_led_avail && num_rgb_groups != 0)
> > + asus->kbd_led.groups = kbd_rgb_mode_groups;
>
> Can't this be placed into the block above?
>
> >
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners)) {
> > rv = led_classdev_register(&asus->platform_device->dev,
> > &asus->kbd_led);
> > - if (rv)
> > + if (rv) {
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
>
> Please use scoped_guard() so you don't need to call unlock yourself.
>
> Unrelated to this patch, I'd also simplify error label by adding return 0
> before it so the code after the label doesn't need if (rv) check.
>
> > goto error;
> > + }
> > + asus->kbd_led_registered = true;
> > +
> > + if (asus->kbd_led_wk >= 0) {
> > + list_for_each_entry(listener, &asus_ref.listeners, list)
> > + listener->brightness_set(listener, asus->kbd_led_wk);
> > + }
> > }
> > + asus_ref.asus = asus;
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> >
> > if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
> > && (asus->driver->quirks->wapf > 0)) {
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index 8a515179113d..f13a701f47a8 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -163,11 +163,19 @@ enum asus_ally_mcu_hack {
> > ASUS_WMI_ALLY_MCU_HACK_DISABLED,
> > };
> >
> > +struct asus_hid_listener {
> > + struct list_head list;
> > + void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > +};
>
> Please add kerneldoc to this struct and the exported functions.
>
> > +
> > #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > void set_ally_mcu_powersave(bool enabled);
> > int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > +
> > +int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > +void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > #else
> > static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > {
> > @@ -184,6 +192,14 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
> > {
> > return -ENODEV;
> > }
> > +
> > +static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > +{
> > + return -ENODEV;
> > +}
> > +static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > +{
> > +}
> > #endif
> >
> > /* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> >
>
> --
> i.
>
>
^ permalink raw reply
* Re: [PATCH v6 6/7] platform/x86: asus-wmi: add keyboard brightness event handler
From: Antheas Kapenekakis @ 2025-10-15 15:38 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <4e4af3e9-26d3-ad03-7868-7fd7dbd541f3@linux.intel.com>
On Wed, 15 Oct 2025 at 14:19, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
>
> > Currenlty, the keyboard brightness control of Asus WMI keyboards is
>
> There's a typo here but preferrably avoid "currently" altogether where
> possible.
>
> > handled in the kernel, which leads to the shortcut going from
> > brightness 0, to 1, to 2, and 3.
> >
> > However, for HID keyboards it is exposed as a key and handled by the
> > user's desktop environment. For the toggle button, this means that
> > brightness control becomes on/off. In addition, in the absence of a
> > DE, the keyboard brightness does not work.
> >
> > Therefore, expose an event handler for the keyboard brightness control
> > which can then be used by hid-asus.
> >
> > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > Tested-by: Luke D. Jones <luke@ljones.dev>
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> > drivers/platform/x86/asus-wmi.c | 41 +++++++++++++++++++++-
> > include/linux/platform_data/x86/asus-wmi.h | 13 +++++++
> > 2 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index a2a7cd61fd59..58407a3b6d41 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -1579,6 +1579,45 @@ void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > }
> > EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
> >
> > +static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
> > +
> > +int asus_hid_event(enum asus_hid_event event)
> > +{
> > + unsigned long flags;
> > + int brightness;
> > +
> > + spin_lock_irqsave(&asus_ref.lock, flags);
> > + if (!asus_ref.asus || !asus_ref.asus->kbd_led_registered) {
>
> Please add a local variable for asus_ref.asus. Check other
> patches/functions too if its use is repeated in some function many times,
> the local var seems to be in order.
>
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
>
> Use guard() instead.
>
> > + return -EBUSY;
> > + }
> > + brightness = asus_ref.asus->kbd_led_wk;
> > +
> > + switch (event) {
> > + case ASUS_EV_BRTUP:
> > + brightness += 1;
> > + break;
> > + case ASUS_EV_BRTDOWN:
> > + brightness -= 1;
> > + break;
> > + case ASUS_EV_BRTTOGGLE:
> > + if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > + brightness = 0;
> > + else
> > + brightness += 1;
> > + break;
> > + }
> > +
> > + do_kbd_led_set(&asus_ref.asus->kbd_led, brightness);
> > + led_classdev_notify_brightness_hw_changed(&asus_ref.asus->kbd_led,
> > + asus_ref.asus->kbd_led_wk);
> > +
> > + spin_unlock_irqrestore(&asus_ref.lock, flags);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > +
> > /*
> > * These functions actually update the LED's, and are called from a
> > * workqueue. By doing this as separate work rather than when the LED
> > @@ -1878,7 +1917,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > asus->kbd_led.brightness_set = kbd_led_set;
> > asus->kbd_led.brightness_get = kbd_led_get;
> > - asus->kbd_led.max_brightness = 3;
> > + asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> >
> > if (asus->kbd_led_avail)
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index 1f85d76387a8..e78e0fbccede 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -168,6 +168,14 @@ struct asus_hid_listener {
> > void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > };
> >
> > +enum asus_hid_event {
> > + ASUS_EV_BRTUP,
> > + ASUS_EV_BRTDOWN,
> > + ASUS_EV_BRTTOGGLE,
>
> Where does "BRT" come from. To me it doesn't associate with brightness
> (might be due to me being non-native). If there's a good reason why it's
> that way, fine but otherwise I suggest changing it so that it becomes
> easier to understand.
>
> It's not a big problem as is because the context in the code above allows
> decrypting the meaning but without the other names, I'd have been totally
> lost what it means.
Comes from e9809c0b9670 ("asus-wmi: add keyboard backlight support")
I matched it to the driver, other alternative is KBDILLUM. I will keep
it as BRT for now to match the current driver.
> > +};
> > +
> > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > +
> > #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > void set_ally_mcu_powersave(bool enabled);
> > @@ -176,6 +184,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> >
> > int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > +int asus_hid_event(enum asus_hid_event event);
> > #else
> > static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > {
> > @@ -200,6 +209,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > {
> > }
> > +static inline int asus_hid_event(enum asus_hid_event event)
> > +{
> > + return -ENODEV;
> > +}
> > #endif
> >
> > #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
> >
>
> --
> i.
>
>
^ permalink raw reply
* Re: [PATCH 0/2] HID: multitouch: fix sticky-fingers quirks
From: Jiri Kosina @ 2025-10-15 15:27 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Shuah Khan, Peter Hutterer, linux-input, linux-kernel,
linux-kselftest, stable
In-Reply-To: <20251008-fix-sticky-fingers-v1-0-760f1f26fce3@kernel.org>
On Wed, 8 Oct 2025, Benjamin Tissoires wrote:
> According to Peter, we've had for a very long time an issue on some
> mutltiouch touchpads where the fingers were stuck in a scrolling mode,
> or 3 fingers gesture mode. I was unable to debug it because it was
> rather hard to reproduce.
>
> Recently, some people raised the issue again on libinput, and this time
> added a recording of the actual bug.
>
> It turns out that the sticky finger quirk that was introduced back in
> 2017 was only checking the last report, and that those missing releases
> also happen when moving from 3 to 1 finger (only 1 is released instead
> of 2).
>
> This solution seems to me to be the most sensible, because we could also
> add the NSMU quirk to win8 multitouch touchpads, but this would involve
> a lot more computations at each report for rather annoying corner cases.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/1194
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Benjamin Tissoires (2):
> HID: multitouch: fix sticky fingers
> selftests/hid: add tests for missing release on the Dell Synaptics
>
> drivers/hid/hid-multitouch.c | 27 ++++++-----
> .../testing/selftests/hid/tests/test_multitouch.py | 55 ++++++++++++++++++++++
> 2 files changed, 69 insertions(+), 13 deletions(-)
Now queued in for-6.18/upstream-fixes. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: multitouch: fix name of Stylus input devices
From: Jiri Kosina @ 2025-10-15 15:17 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: Benjamin Tissoires, Mika Westerberg, Tero Kristo, linux-input,
linux-kernel, kernel-dev
In-Reply-To: <20251008-hid_multitouch_stylus-v1-1-9f43f7e79195@igalia.com>
On Wed, 8 Oct 2025, Thadeu Lima de Souza Cascardo wrote:
> HID_DG_PEN devices should have a suffix of "Stylus", as pointed out by
> commit c0ee1d571626 ("HID: hid-input: Add suffix also for HID_DG_PEN").
> However, on multitouch devices, these suffixes may be overridden. Before
> that commit, HID_DG_PEN devices would get the "Stylus" suffix, but after
> that, multitouch would override them to have an "UNKNOWN" suffix. Just add
> HID_DG_PEN to the list of non-overriden suffixes in multitouch.
>
> Before this fix:
>
> [ 0.470981] input: ELAN9008:00 04F3:2E14 UNKNOWN as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-16/i2c-ELAN9008:00/0018:04F3:2E14.0001/input/input8
> ELAN9008:00 04F3:2E14 UNKNOWN
>
> After this fix:
>
> [ 0.474332] input: ELAN9008:00 04F3:2E14 Stylus as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-16/i2c-ELAN9008:00/0018:04F3:2E14.0001/input/input8
>
> ELAN9008:00 04F3:2E14 Stylus
>
> Fixes: c0ee1d571626 ("HID: hid-input: Add suffix also for HID_DG_PEN")
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: hid-input: only ignore 0 battery events for digitizers
From: Jiri Kosina @ 2025-10-15 15:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, 卢国宏, kenalba,
linux-input, linux-kernel
In-Reply-To: <c5b52grvciabpcgavhjqximqqq6fczowgvmckke6aflq72mzyv@gzzkyt25xygc>
On Tue, 14 Oct 2025, Dmitry Torokhov wrote:
> Commit 581c4484769e ("HID: input: map digitizer battery usage") added
> handling of battery events for digitizers (typically for batteries
> presented in stylii). Digitizers typically report correct battery levels
> only when stylus is actively touching the surface, and in other cases
> they may report battery level of 0. To avoid confusing consumers of the
> battery information the code was added to filer out reports with 0
> battery levels.
>
> However there exist other kinds of devices that may legitimately report
> 0 battery levels. Fix this by filtering out 0-level reports only for
> digitizer usages, and continue reporting them for other kinds of devices
> (Smart Batteries, etc).
>
> Reported-by: 卢国宏 <luguohong@xiaomi.com>
> Fixes: 581c4484769e ("HID: input: map digitizer battery usage")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> v2: rebased on top of linux-next, dropped Tested-by: tag
Applied, thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
From: Jiri Kosina @ 2025-10-15 14:08 UTC (permalink / raw)
To: Zhang, Lixu
Cc: srinivas pandruvada, linux-input@vger.kernel.org,
benjamin.tissoires@redhat.com, Wang, Selina
In-Reply-To: <SJ0PR11MB56134BF6170CD82795EC2F1B93E8A@SJ0PR11MB5613.namprd11.prod.outlook.com>
On Wed, 15 Oct 2025, Zhang, Lixu wrote:
> I personally prefer to have this patch merged before the patch series
> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a merge
> conflict exists between this patch and that patch series. If this patch
> is merged first, it would be more convenient for others to backport this
> patch to other kernel versions. BTW, in this case, the patch series
> [PATCH 0/6] HID: intel-ish-hid: Various power management would need to
> be rebased. Do I need to send a v2 patch series for the rebase?
We can definitely do that. The question is whether this one should go in
right now for 6.18 still, or we queue both for 6.19.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
From: syzbot @ 2025-10-15 12:40 UTC (permalink / raw)
To: bentiss, jikos, linux-input, linux-kernel, linux-usb, oneukum,
syzkaller-bugs
In-Reply-To: <35e0f3f4-def7-4a94-a982-549b6203c6e2@suse.com>
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: slab-out-of-bounds Read in mcp2221_raw_event
==================================================================
BUG: KASAN: slab-out-of-bounds in mcp2221_raw_event+0x1276/0x12a0 drivers/hid/hid-mcp2221.c:977
Read of size 1 at addr ffff88805c577fff by task dhcpcd-run-hook/6714
CPU: 0 UID: 0 PID: 6714 Comm: dhcpcd-run-hook Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:378 [inline]
print_report+0xcd/0x630 mm/kasan/report.c:482
kasan_report+0xe0/0x110 mm/kasan/report.c:595
mcp2221_raw_event+0x1276/0x12a0 drivers/hid/hid-mcp2221.c:977
__hid_input_report.constprop.0+0x314/0x450 drivers/hid/hid-core.c:2139
hid_irq_in+0x35e/0x870 drivers/hid/usbhid/hid-core.c:286
__usb_hcd_giveback_urb+0x38b/0x610 drivers/usb/core/hcd.c:1661
usb_hcd_giveback_urb+0x39b/0x450 drivers/usb/core/hcd.c:1745
dummy_timer+0x1809/0x3a00 drivers/usb/gadget/udc/dummy_hcd.c:1995
__run_hrtimer kernel/time/hrtimer.c:1777 [inline]
__hrtimer_run_queues+0x202/0xad0 kernel/time/hrtimer.c:1841
hrtimer_run_softirq+0x17d/0x350 kernel/time/hrtimer.c:1858
handle_softirqs+0x219/0x8e0 kernel/softirq.c:622
__do_softirq kernel/softirq.c:656 [inline]
invoke_softirq kernel/softirq.c:496 [inline]
__irq_exit_rcu+0x109/0x170 kernel/softirq.c:723
irq_exit_rcu+0x9/0x30 kernel/softirq.c:739
instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1052 [inline]
sysvec_apic_timer_interrupt+0xa4/0xc0 arch/x86/kernel/apic/apic.c:1052
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:697
RIP: 0010:mt_find+0x99a/0xa20 lib/maple_tree.c:6502
Code: e9 f4 fa ff ff 31 db e9 9c fd ff ff 4c 89 ff e8 dc 51 c6 f6 e9 d4 f8 ff ff e8 a2 51 c6 f6 e9 8f f8 ff ff e8 88 ca 5d f6 31 db <e8> 81 ca 5d f6 48 85 db 0f 85 80 f9 ff ff e9 df fc ff ff e8 6e ca
RSP: 0018:ffffc9000457fbc8 EFLAGS: 00000292
RAX: 0000000000000000 RBX: ffff88803166c8c0 RCX: ffffffff8b5f6970
RDX: ffff888033084900 RSI: ffffffff8b5f6e74 RDI: 0000000000000005
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
R13: ffffc9000457fd08 R14: 0000000000000300 R15: 0000000000000002
find_vma+0xbf/0x140 mm/mmap.c:909
lock_mm_and_find_vma+0x62/0x6e0 mm/mmap_lock.c:431
do_user_addr_fault+0x2ac/0x1370 arch/x86/mm/fault.c:1359
handle_page_fault arch/x86/mm/fault.c:1476 [inline]
exc_page_fault+0x64/0xc0 arch/x86/mm/fault.c:1532
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:618
RIP: 0010:__put_user_4+0xd/0x20 arch/x86/lib/putuser.S:94
Code: 66 89 01 31 c9 0f 01 ca c3 cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 48 89 cb 48 c1 fb 3f 48 09 d9 0f 01 cb <89> 01 31 c9 0f 01 ca c3 cc cc cc cc 0f 1f 80 00 00 00 00 90 90 90
RSP: 0018:ffffc9000457fef8 EFLAGS: 00050206
RAX: 0000000000001a3a RBX: 0000000000000000 RCX: 00007f8b8be0df50
RDX: dffffc0000000000 RSI: ffffffff8dadd91d RDI: ffff888033084fe0
RBP: ffffc9000457ff48 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000001 R12: ffff88802b8fc900
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
schedule_tail+0xb0/0xe0 kernel/sched/core.c:5260
ret_from_fork+0x25/0x7d0 arch/x86/kernel/process.c:154
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Allocated by task 6476:
kasan_save_stack+0x33/0x60 mm/kasan/common.c:56
kasan_save_track+0x14/0x30 mm/kasan/common.c:77
unpoison_slab_object mm/kasan/common.c:342 [inline]
__kasan_slab_alloc+0x89/0x90 mm/kasan/common.c:368
kasan_slab_alloc include/linux/kasan.h:252 [inline]
slab_post_alloc_hook mm/slub.c:4955 [inline]
slab_alloc_node mm/slub.c:5265 [inline]
kmem_cache_alloc_node_noprof+0x28a/0x770 mm/slub.c:5317
kmalloc_reserve+0x18b/0x2c0 net/core/skbuff.c:579
__alloc_skb+0x166/0x380 net/core/skbuff.c:670
alloc_skb include/linux/skbuff.h:1383 [inline]
alloc_uevent_skb+0x7d/0x210 lib/kobject_uevent.c:289
uevent_net_broadcast_untagged lib/kobject_uevent.c:326 [inline]
kobject_uevent_net_broadcast lib/kobject_uevent.c:410 [inline]
kobject_uevent_env+0xca4/0x1870 lib/kobject_uevent.c:608
device_del+0x623/0x9f0 drivers/base/core.c:3896
gpiochip_remove+0x5aa/0x6d0 drivers/gpio/gpiolib.c:1303
release_nodes+0x116/0x240 drivers/base/devres.c:505
devres_release_group+0x1c1/0x300 drivers/base/devres.c:692
hid_device_remove+0x107/0x260 drivers/hid/hid-core.c:2836
device_remove+0xcb/0x170 drivers/base/dd.c:569
__device_release_driver drivers/base/dd.c:1274 [inline]
device_release_driver_internal+0x44b/0x620 drivers/base/dd.c:1297
bus_remove_device+0x22f/0x420 drivers/base/bus.c:579
device_del+0x396/0x9f0 drivers/base/core.c:3878
hid_remove_device drivers/hid/hid-core.c:3008 [inline]
hid_destroy_device+0x19c/0x240 drivers/hid/hid-core.c:3030
usbhid_disconnect+0xa0/0xe0 drivers/hid/usbhid/hid-core.c:1462
usb_unbind_interface+0x1dd/0x9e0 drivers/usb/core/driver.c:458
device_remove drivers/base/dd.c:571 [inline]
device_remove+0x125/0x170 drivers/base/dd.c:563
__device_release_driver drivers/base/dd.c:1274 [inline]
device_release_driver_internal+0x44b/0x620 drivers/base/dd.c:1297
bus_remove_device+0x22f/0x420 drivers/base/bus.c:579
device_del+0x396/0x9f0 drivers/base/core.c:3878
usb_disable_device+0x355/0x7d0 drivers/usb/core/message.c:1418
usb_disconnect+0x2e1/0x9c0 drivers/usb/core/hub.c:2344
hub_port_connect drivers/usb/core/hub.c:5406 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
port_event drivers/usb/core/hub.c:5870 [inline]
hub_event+0x1c81/0x4fe0 drivers/usb/core/hub.c:5952
process_one_work+0x9cf/0x1b70 kernel/workqueue.c:3263
process_scheduled_works kernel/workqueue.c:3346 [inline]
worker_thread+0x6c8/0xf10 kernel/workqueue.c:3427
kthread+0x3c5/0x780 kernel/kthread.c:463
ret_from_fork+0x675/0x7d0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
Freed by task 5183:
kasan_save_stack+0x33/0x60 mm/kasan/common.c:56
kasan_save_track+0x14/0x30 mm/kasan/common.c:77
__kasan_save_free_info+0x3b/0x60 mm/kasan/generic.c:587
kasan_save_free_info mm/kasan/kasan.h:406 [inline]
poison_slab_object mm/kasan/common.c:252 [inline]
__kasan_slab_free+0x5f/0x80 mm/kasan/common.c:284
kasan_slab_free include/linux/kasan.h:234 [inline]
slab_free_hook mm/slub.c:2523 [inline]
slab_free mm/slub.c:6611 [inline]
kmem_cache_free+0x2d4/0x6c0 mm/slub.c:6721
skb_kfree_head net/core/skbuff.c:1046 [inline]
skb_kfree_head net/core/skbuff.c:1043 [inline]
skb_free_head+0x1b7/0x210 net/core/skbuff.c:1060
skb_release_data+0x795/0x9e0 net/core/skbuff.c:1087
skb_release_all net/core/skbuff.c:1152 [inline]
__kfree_skb net/core/skbuff.c:1166 [inline]
consume_skb net/core/skbuff.c:1398 [inline]
consume_skb+0xbf/0x100 net/core/skbuff.c:1392
netlink_recvmsg+0x5b9/0xa90 net/netlink/af_netlink.c:1974
sock_recvmsg_nosec net/socket.c:1078 [inline]
sock_recvmsg+0x1f9/0x250 net/socket.c:1100
____sys_recvmsg+0x218/0x6b0 net/socket.c:2850
___sys_recvmsg+0x114/0x1a0 net/socket.c:2892
__sys_recvmsg+0x16a/0x220 net/socket.c:2925
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The buggy address belongs to the object at ffff88805c577a80
which belongs to the cache skbuff_small_head of size 704
The buggy address is located 703 bytes to the right of
allocated 704-byte region [ffff88805c577a80, ffff88805c577d40)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5c574
head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 00fff00000000040 ffff888140edbb40 ffffea0001f37a00 dead000000000004
raw: 0000000000000000 0000000000130013 00000000f5000000 0000000000000000
head: 00fff00000000040 ffff888140edbb40 ffffea0001f37a00 dead000000000004
head: 0000000000000000 0000000000130013 00000000f5000000 0000000000000000
head: 00fff00000000002 ffffea0001715d01 00000000ffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000004
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 2, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 6343, tgid 6343 (syz-executor), ts 113786247405, free_ts 94133356886
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1c0/0x230 mm/page_alloc.c:1850
prep_new_page mm/page_alloc.c:1858 [inline]
get_page_from_freelist+0x10a3/0x3a30 mm/page_alloc.c:3884
__alloc_frozen_pages_noprof+0x25f/0x2470 mm/page_alloc.c:5183
alloc_pages_mpol+0x1fb/0x550 mm/mempolicy.c:2416
alloc_slab_page mm/slub.c:3039 [inline]
allocate_slab mm/slub.c:3212 [inline]
new_slab+0x24a/0x360 mm/slub.c:3266
___slab_alloc+0xdc4/0x1ae0 mm/slub.c:4636
__slab_alloc.constprop.0+0x63/0x110 mm/slub.c:4755
__slab_alloc_node mm/slub.c:4831 [inline]
slab_alloc_node mm/slub.c:5253 [inline]
kmem_cache_alloc_node_noprof+0x43c/0x770 mm/slub.c:5317
kmalloc_reserve+0x18b/0x2c0 net/core/skbuff.c:579
__alloc_skb+0x166/0x380 net/core/skbuff.c:670
alloc_skb include/linux/skbuff.h:1383 [inline]
nlmsg_new include/net/netlink.h:1055 [inline]
mpls_netconf_notify_devconf+0x4a/0x110 net/mpls/af_mpls.c:1189
mpls_dev_sysctl_register+0x1c9/0x2a0 net/mpls/af_mpls.c:1409
mpls_add_dev net/mpls/af_mpls.c:1460 [inline]
mpls_dev_notify+0x4ab/0xa20 net/mpls/af_mpls.c:1600
notifier_call_chain+0xbc/0x410 kernel/notifier.c:85
call_netdevice_notifiers_info+0xbe/0x140 net/core/dev.c:2229
call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
call_netdevice_notifiers net/core/dev.c:2281 [inline]
register_netdevice+0x182e/0x2270 net/core/dev.c:11332
page last free pid 6136 tgid 6136 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1394 [inline]
__free_frozen_pages+0x7df/0x1160 mm/page_alloc.c:2906
discard_slab mm/slub.c:3310 [inline]
__put_partials+0x130/0x170 mm/slub.c:3857
qlink_free mm/kasan/quarantine.c:163 [inline]
qlist_free_all+0x4d/0x120 mm/kasan/quarantine.c:179
kasan_quarantine_reduce+0x195/0x1e0 mm/kasan/quarantine.c:286
__kasan_slab_alloc+0x69/0x90 mm/kasan/common.c:352
kasan_slab_alloc include/linux/kasan.h:252 [inline]
slab_post_alloc_hook mm/slub.c:4955 [inline]
slab_alloc_node mm/slub.c:5265 [inline]
__do_kmalloc_node mm/slub.c:5626 [inline]
__kmalloc_noprof+0x2e8/0x880 mm/slub.c:5639
kmalloc_noprof include/linux/slab.h:961 [inline]
tomoyo_realpath_from_path+0xc2/0x6e0 security/tomoyo/realpath.c:251
tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
tomoyo_path_perm+0x274/0x460 security/tomoyo/file.c:822
security_inode_getattr+0x116/0x290 security/security.c:2416
vfs_getattr fs/stat.c:259 [inline]
vfs_fstat+0x4b/0xe0 fs/stat.c:281
__do_sys_newfstat+0x87/0x100 fs/stat.c:555
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Memory state around the buggy address:
ffff88805c577e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88805c577f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88805c577f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff88805c578000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88805c578080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
----------------
Code disassembly (best guess):
0: e9 f4 fa ff ff jmp 0xfffffaf9
5: 31 db xor %ebx,%ebx
7: e9 9c fd ff ff jmp 0xfffffda8
c: 4c 89 ff mov %r15,%rdi
f: e8 dc 51 c6 f6 call 0xf6c651f0
14: e9 d4 f8 ff ff jmp 0xfffff8ed
19: e8 a2 51 c6 f6 call 0xf6c651c0
1e: e9 8f f8 ff ff jmp 0xfffff8b2
23: e8 88 ca 5d f6 call 0xf65dcab0
28: 31 db xor %ebx,%ebx
* 2a: e8 81 ca 5d f6 call 0xf65dcab0 <-- trapping instruction
2f: 48 85 db test %rbx,%rbx
32: 0f 85 80 f9 ff ff jne 0xfffff9b8
38: e9 df fc ff ff jmp 0xfffffd1c
3d: e8 .byte 0xe8
3e: 6e outsb %ds:(%rsi),(%dx)
3f: ca .byte 0xca
Tested on:
commit: 3a866087 Linux 6.18-rc1
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1721d542580000
kernel config: https://syzkaller.appspot.com/x/.config?x=f3e7b5a3627a90dd
dashboard link: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1004952f980000
^ permalink raw reply
* Re: [PATCH v6 6/7] platform/x86: asus-wmi: add keyboard brightness event handler
From: Ilpo Järvinen @ 2025-10-15 12:18 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, Denis Benato
In-Reply-To: <20251013201535.6737-7-lkml@antheas.dev>
On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> Currenlty, the keyboard brightness control of Asus WMI keyboards is
There's a typo here but preferrably avoid "currently" altogether where
possible.
> handled in the kernel, which leads to the shortcut going from
> brightness 0, to 1, to 2, and 3.
>
> However, for HID keyboards it is exposed as a key and handled by the
> user's desktop environment. For the toggle button, this means that
> brightness control becomes on/off. In addition, in the absence of a
> DE, the keyboard brightness does not work.
>
> Therefore, expose an event handler for the keyboard brightness control
> which can then be used by hid-asus.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Tested-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/platform/x86/asus-wmi.c | 41 +++++++++++++++++++++-
> include/linux/platform_data/x86/asus-wmi.h | 13 +++++++
> 2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index a2a7cd61fd59..58407a3b6d41 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1579,6 +1579,45 @@ void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> }
> EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
>
> +static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
> +
> +int asus_hid_event(enum asus_hid_event event)
> +{
> + unsigned long flags;
> + int brightness;
> +
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + if (!asus_ref.asus || !asus_ref.asus->kbd_led_registered) {
Please add a local variable for asus_ref.asus. Check other
patches/functions too if its use is repeated in some function many times,
the local var seems to be in order.
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
Use guard() instead.
> + return -EBUSY;
> + }
> + brightness = asus_ref.asus->kbd_led_wk;
> +
> + switch (event) {
> + case ASUS_EV_BRTUP:
> + brightness += 1;
> + break;
> + case ASUS_EV_BRTDOWN:
> + brightness -= 1;
> + break;
> + case ASUS_EV_BRTTOGGLE:
> + if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> + brightness = 0;
> + else
> + brightness += 1;
> + break;
> + }
> +
> + do_kbd_led_set(&asus_ref.asus->kbd_led, brightness);
> + led_classdev_notify_brightness_hw_changed(&asus_ref.asus->kbd_led,
> + asus_ref.asus->kbd_led_wk);
> +
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_event);
> +
> /*
> * These functions actually update the LED's, and are called from a
> * workqueue. By doing this as separate work rather than when the LED
> @@ -1878,7 +1917,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> asus->kbd_led.brightness_set = kbd_led_set;
> asus->kbd_led.brightness_get = kbd_led_get;
> - asus->kbd_led.max_brightness = 3;
> + asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
>
> if (asus->kbd_led_avail)
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 1f85d76387a8..e78e0fbccede 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -168,6 +168,14 @@ struct asus_hid_listener {
> void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> };
>
> +enum asus_hid_event {
> + ASUS_EV_BRTUP,
> + ASUS_EV_BRTDOWN,
> + ASUS_EV_BRTTOGGLE,
Where does "BRT" come from. To me it doesn't associate with brightness
(might be due to me being non-native). If there's a good reason why it's
that way, fine but otherwise I suggest changing it so that it becomes
easier to understand.
It's not a big problem as is because the context in the code above allows
decrypting the meaning but without the other names, I'd have been totally
lost what it means.
> +};
> +
> +#define ASUS_EV_MAX_BRIGHTNESS 3
> +
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> void set_ally_mcu_powersave(bool enabled);
> @@ -176,6 +184,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
>
> int asus_hid_register_listener(struct asus_hid_listener *cdev);
> void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> +int asus_hid_event(enum asus_hid_event event);
> #else
> static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> {
> @@ -200,6 +209,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> {
> }
> +static inline int asus_hid_event(enum asus_hid_event event)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
>
--
i.
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
From: Oliver Neukum @ 2025-10-15 12:17 UTC (permalink / raw)
To: syzbot, bentiss, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <68ee6a3f.050a0220.91a22.021b.GAE@google.com>
[-- Attachment #1: Type: text/plain, Size: 14626 bytes --]
#syz test: upstream 3a8660878839
On 14.10.25 17:20, syzbot wrote:
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit: 3a8660878839 Linux 6.18-rc1
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12a705e2580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f3e7b5a3627a90dd
> dashboard link: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
> compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=132ebb34580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=140fe52f980000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/e767e8931970/disk-3a866087.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/4cb12bdcfcea/vmlinux-3a866087.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/b08acfae954d/bzImage-3a866087.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+1018672fe70298606e5f@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in mcp2221_raw_event+0x1070/0x10a0 drivers/hid/hid-mcp2221.c:948
> Read of size 1 at addr ffff8880721cbfff by task kworker/0:7/6094
>
> CPU: 0 UID: 0 PID: 6094 Comm: kworker/0:7 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:378 [inline]
> print_report+0xcd/0x630 mm/kasan/report.c:482
> kasan_report+0xe0/0x110 mm/kasan/report.c:595
> mcp2221_raw_event+0x1070/0x10a0 drivers/hid/hid-mcp2221.c:948
> __hid_input_report.constprop.0+0x314/0x450 drivers/hid/hid-core.c:2139
> hid_irq_in+0x35e/0x870 drivers/hid/usbhid/hid-core.c:286
> __usb_hcd_giveback_urb+0x38b/0x610 drivers/usb/core/hcd.c:1661
> usb_hcd_giveback_urb+0x39b/0x450 drivers/usb/core/hcd.c:1745
> dummy_timer+0x1809/0x3a00 drivers/usb/gadget/udc/dummy_hcd.c:1995
> __run_hrtimer kernel/time/hrtimer.c:1777 [inline]
> __hrtimer_run_queues+0x202/0xad0 kernel/time/hrtimer.c:1841
> hrtimer_run_softirq+0x17d/0x350 kernel/time/hrtimer.c:1858
> handle_softirqs+0x219/0x8e0 kernel/softirq.c:622
> __do_softirq kernel/softirq.c:656 [inline]
> invoke_softirq kernel/softirq.c:496 [inline]
> __irq_exit_rcu+0x109/0x170 kernel/softirq.c:723
> irq_exit_rcu+0x9/0x30 kernel/softirq.c:739
> instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1052 [inline]
> sysvec_apic_timer_interrupt+0xa4/0xc0 arch/x86/kernel/apic/apic.c:1052
> </IRQ>
> <TASK>
> asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:697
> RIP: 0010:kasan_check_range+0x12/0x1b0 mm/kasan/generic.c:199
> Code: 00 00 00 00 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 85 f6 0f 84 64 01 00 00 48 89 f8 41 54 <44> 0f b6 c2 48 01 f0 55 53 0f 82 d7 00 00 00 eb 0f cc cc cc 48 b8
> RSP: 0018:ffffc900037b6b60 EFLAGS: 00000202
> RAX: ffff888077da86b0 RBX: ffff888077da8668 RCX: ffffffff819803ae
> RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888077da86b0
> RBP: ffff888077da86b0 R08: 0000000000000002 R09: 0000000000000000
> R10: ffff888077da866f R11: 0000000000000000 R12: ffffffff8c6df2a0
> R13: ffffffff9ae57620 R14: 0000000000000000 R15: ffff888026709978
> instrument_atomic_write include/linux/instrumented.h:82 [inline]
> atomic_set include/linux/atomic/atomic-instrumented.h:67 [inline]
> osq_lock_init include/linux/osq_lock.h:25 [inline]
> __mutex_init+0xae/0x120 kernel/locking/mutex.c:53
> i2c_register_adapter+0x15d/0x1370 drivers/i2c/i2c-core-base.c:1544
> i2c_add_adapter drivers/i2c/i2c-core-base.c:1673 [inline]
> i2c_add_adapter+0x10a/0x1b0 drivers/i2c/i2c-core-base.c:1653
> devm_i2c_add_adapter+0x1b/0x90 drivers/i2c/i2c-core-base.c:1845
> mcp2221_probe+0x5f1/0xc50 drivers/hid/hid-mcp2221.c:1289
> __hid_device_probe drivers/hid/hid-core.c:2775 [inline]
> hid_device_probe+0x5ba/0x8d0 drivers/hid/hid-core.c:2812
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> hid_add_device+0x31b/0x5c0 drivers/hid/hid-core.c:2951
> usbhid_probe+0xd38/0x13f0 drivers/hid/usbhid/hid-core.c:1435
> usb_probe_interface+0x303/0xa40 drivers/usb/core/driver.c:396
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> usb_set_configuration+0x1187/0x1e20 drivers/usb/core/message.c:2210
> usb_generic_driver_probe+0xb1/0x110 drivers/usb/core/generic.c:250
> usb_probe_device+0xef/0x3e0 drivers/usb/core/driver.c:291
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> usb_new_device+0xd07/0x1a60 drivers/usb/core/hub.c:2694
> hub_port_connect drivers/usb/core/hub.c:5566 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
> port_event drivers/usb/core/hub.c:5870 [inline]
> hub_event+0x2f34/0x4fe0 drivers/usb/core/hub.c:5952
> process_one_work+0x9cf/0x1b70 kernel/workqueue.c:3263
> process_scheduled_works kernel/workqueue.c:3346 [inline]
> worker_thread+0x6c8/0xf10 kernel/workqueue.c:3427
> kthread+0x3c5/0x780 kernel/kthread.c:463
> ret_from_fork+0x675/0x7d0 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> </TASK>
>
> Allocated by task 5918:
> kasan_save_stack+0x33/0x60 mm/kasan/common.c:56
> kasan_save_track+0x14/0x30 mm/kasan/common.c:77
> poison_kmalloc_redzone mm/kasan/common.c:400 [inline]
> __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:417
> kmalloc_noprof include/linux/slab.h:957 [inline]
> kzalloc_noprof include/linux/slab.h:1094 [inline]
> ipv6_add_addr+0x4e3/0x1fe0 net/ipv6/addrconf.c:1120
> add_addr+0xde/0x350 net/ipv6/addrconf.c:3201
> add_v4_addrs+0x642/0x980 net/ipv6/addrconf.c:3263
> addrconf_gre_config net/ipv6/addrconf.c:3545 [inline]
> addrconf_init_auto_addrs+0x51a/0x810 net/ipv6/addrconf.c:3559
> addrconf_notify+0xe93/0x19e0 net/ipv6/addrconf.c:3740
> notifier_call_chain+0xbc/0x410 kernel/notifier.c:85
> call_netdevice_notifiers_info+0xbe/0x140 net/core/dev.c:2229
> call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
> call_netdevice_notifiers net/core/dev.c:2281 [inline]
> __dev_notify_flags+0x12c/0x2e0 net/core/dev.c:9676
> netif_change_flags+0x108/0x160 net/core/dev.c:9705
> do_setlink.constprop.0+0xb53/0x4380 net/core/rtnetlink.c:3151
> rtnl_changelink net/core/rtnetlink.c:3769 [inline]
> __rtnl_newlink net/core/rtnetlink.c:3928 [inline]
> rtnl_newlink+0x1446/0x2000 net/core/rtnetlink.c:4065
> rtnetlink_rcv_msg+0x95e/0xe90 net/core/rtnetlink.c:6954
> netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1346
> netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1896
> sock_sendmsg_nosec net/socket.c:727 [inline]
> __sock_sendmsg net/socket.c:742 [inline]
> __sys_sendto+0x4a3/0x520 net/socket.c:2244
> __do_sys_sendto net/socket.c:2251 [inline]
> __se_sys_sendto net/socket.c:2247 [inline]
> __x64_sys_sendto+0xe0/0x1c0 net/socket.c:2247
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> The buggy address belongs to the object at ffff8880721cbc00
> which belongs to the cache kmalloc-cg-512 of size 512
> The buggy address is located 583 bytes to the right of
> allocated 440-byte region [ffff8880721cbc00, ffff8880721cbdb8)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x721c8
> head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
> page_type: f5(slab)
> raw: 00fff00000000040 ffff88813ff30140 ffffea0001e68c00 dead000000000002
> raw: 0000000000000000 0000000000100010 00000000f5000000 0000000000000000
> head: 00fff00000000040 ffff88813ff30140 ffffea0001e68c00 dead000000000002
> head: 0000000000000000 0000000000100010 00000000f5000000 0000000000000000
> head: 00fff00000000002 ffffea0001c87201 00000000ffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000004
> page dumped because: kasan: bad access detected
> page_owner tracks the page as allocated
> page last allocated via order 2, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 5796, tgid 5796 (sshd-session), ts 52056965840, free_ts 15121629475
> set_page_owner include/linux/page_owner.h:32 [inline]
> post_alloc_hook+0x1c0/0x230 mm/page_alloc.c:1850
> prep_new_page mm/page_alloc.c:1858 [inline]
> get_page_from_freelist+0x10a3/0x3a30 mm/page_alloc.c:3884
> __alloc_frozen_pages_noprof+0x25f/0x2470 mm/page_alloc.c:5183
> alloc_pages_mpol+0x1fb/0x550 mm/mempolicy.c:2416
> alloc_slab_page mm/slub.c:3039 [inline]
> allocate_slab mm/slub.c:3212 [inline]
> new_slab+0x24a/0x360 mm/slub.c:3266
> ___slab_alloc+0xdc4/0x1ae0 mm/slub.c:4636
> __slab_alloc.constprop.0+0x63/0x110 mm/slub.c:4755
> __slab_alloc_node mm/slub.c:4831 [inline]
> slab_alloc_node mm/slub.c:5253 [inline]
> __do_kmalloc_node mm/slub.c:5626 [inline]
> __kmalloc_node_track_caller_noprof+0x4db/0x8a0 mm/slub.c:5736
> kmalloc_reserve+0xef/0x2c0 net/core/skbuff.c:601
> __alloc_skb+0x166/0x380 net/core/skbuff.c:670
> alloc_skb include/linux/skbuff.h:1383 [inline]
> alloc_skb_with_frags+0xe0/0x860 net/core/skbuff.c:6671
> sock_alloc_send_pskb+0x7f9/0x980 net/core/sock.c:2965
> unix_stream_sendmsg+0x39f/0x1340 net/unix/af_unix.c:2455
> sock_sendmsg_nosec net/socket.c:727 [inline]
> __sock_sendmsg net/socket.c:742 [inline]
> sock_write_iter+0x566/0x610 net/socket.c:1195
> new_sync_write fs/read_write.c:593 [inline]
> vfs_write+0x7d3/0x11d0 fs/read_write.c:686
> ksys_write+0x1f8/0x250 fs/read_write.c:738
> page last free pid 1 tgid 1 stack trace:
> reset_page_owner include/linux/page_owner.h:25 [inline]
> free_pages_prepare mm/page_alloc.c:1394 [inline]
> __free_frozen_pages+0x7df/0x1160 mm/page_alloc.c:2906
> __free_pages mm/page_alloc.c:5302 [inline]
> free_contig_range+0x183/0x4b0 mm/page_alloc.c:7146
> destroy_args+0xb69/0x12e0 mm/debug_vm_pgtable.c:958
> debug_vm_pgtable+0x1a32/0x3640 mm/debug_vm_pgtable.c:1345
> do_one_initcall+0x123/0x6e0 init/main.c:1283
> do_initcall_level init/main.c:1345 [inline]
> do_initcalls init/main.c:1361 [inline]
> do_basic_setup init/main.c:1380 [inline]
> kernel_init_freeable+0x5c8/0x920 init/main.c:1593
> kernel_init+0x1c/0x2b0 init/main.c:1483
> ret_from_fork+0x675/0x7d0 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>
> Memory state around the buggy address:
> ffff8880721cbe80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8880721cbf00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff8880721cbf80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff8880721cc000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8880721cc080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
> ----------------
> Code disassembly (best guess):
> 0: 00 00 add %al,(%rax)
> 2: 00 00 add %al,(%rax)
> 4: 0f 1f 40 00 nopl 0x0(%rax)
> 8: 90 nop
> 9: 90 nop
> a: 90 nop
> b: 90 nop
> c: 90 nop
> d: 90 nop
> e: 90 nop
> f: 90 nop
> 10: 90 nop
> 11: 90 nop
> 12: 90 nop
> 13: 90 nop
> 14: 90 nop
> 15: 90 nop
> 16: 90 nop
> 17: 90 nop
> 18: 0f 1f 40 d6 nopl -0x2a(%rax)
> 1c: 48 85 f6 test %rsi,%rsi
> 1f: 0f 84 64 01 00 00 je 0x189
> 25: 48 89 f8 mov %rdi,%rax
> 28: 41 54 push %r12
> * 2a: 44 0f b6 c2 movzbl %dl,%r8d <-- trapping instruction
> 2e: 48 01 f0 add %rsi,%rax
> 31: 55 push %rbp
> 32: 53 push %rbx
> 33: 0f 82 d7 00 00 00 jb 0x110
> 39: eb 0f jmp 0x4a
> 3b: cc int3
> 3c: cc int3
> 3d: cc int3
> 3e: 48 rex.W
> 3f: b8 .byte 0xb8
>
>
> ---
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
[-- Attachment #2: 0001-hid-mcp2221-validate-message-length.patch --]
[-- Type: text/x-patch, Size: 4406 bytes --]
From 41216a0385b9d2ff1f42a860109bba286fe9d28b Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 15 Oct 2025 13:49:05 +0200
Subject: [PATCH] hid-mcp2221: validate message length
The message passed to raw_event is of indeterminate length.
Check for length before accessing members.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/hid/hid-mcp2221.c | 44 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 33603b019f97..d5e9f7ef8ba8 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -849,12 +849,18 @@ static int mcp2221_raw_event(struct hid_device *hdev,
u8 *buf;
struct mcp2221 *mcp = hid_get_drvdata(hdev);
+ if (size <= 0)
+ goto bail;
+
switch (data[0]) {
case MCP2221_I2C_WR_DATA:
case MCP2221_I2C_WR_NO_STOP:
case MCP2221_I2C_RD_DATA:
case MCP2221_I2C_RD_RPT_START:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -866,6 +872,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_I2C_PARAM_OR_STATUS:
+ if (size < 4)
+ goto bail;
switch (data[1]) {
case MCP2221_SUCCESS:
if ((mcp->txbuf[3] == MCP2221_I2C_SET_SPEED) &&
@@ -873,6 +881,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
mcp->status = -EAGAIN;
break;
}
+ if (size < 21)
+ goto bail;
if (data[20] & MCP2221_I2C_MASK_ADDR_NACK) {
mcp->status = -ENXIO;
break;
@@ -889,12 +899,19 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_I2C_GET_DATA:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < 3)
+ goto bail;
if (data[2] == MCP2221_I2C_ADDR_NACK) {
mcp->status = -ENXIO;
break;
}
+ if (size < 4)
+ goto bail;
if (!mcp_get_i2c_eng_state(mcp, data, 2)
&& (data[3] == 0)) {
mcp->status = 0;
@@ -906,7 +923,9 @@ static int mcp2221_raw_event(struct hid_device *hdev,
}
if (data[2] == MCP2221_I2C_READ_COMPL ||
data[2] == MCP2221_I2C_READ_PARTIAL) {
- if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) {
+ if (!mcp->rxbuf ||
+ mcp->rxbuf_idx < 0 || data[3] > 60 ||
+ data[3] > size - 4 ) {
mcp->status = -EINVAL;
break;
}
@@ -925,8 +944,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GPIO_GET:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (mcp->gp_idx < size)
+ goto bail;
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
(data[mcp->gp_idx + 1] == MCP2221_ALT_F_NOT_GPIOD)) {
mcp->status = -ENOENT;
@@ -942,8 +966,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GPIO_SET:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < mcp->gp_idx)
+ goto bail;
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
(data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
mcp->status = -ENOENT;
@@ -958,6 +987,9 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_SET_SRAM_SETTINGS:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -969,8 +1001,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GET_SRAM_SETTINGS:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < 22 + 4)
+ goto bail;
memcpy(&mcp->mode, &data[22], 4);
#if IS_REACHABLE(CONFIG_IIO)
mcp->dac_value = data[6] & GENMASK(4, 0);
@@ -984,6 +1021,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_READ_FLASH_DATA:
+ if (size < 2)
+ goto bail;
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -997,6 +1036,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
#if IS_REACHABLE(CONFIG_IIO)
{
u8 tmp;
+ if (size < 8)
+ goto bail;
/* DAC scale value */
tmp = FIELD_GET(GENMASK(7, 6), data[6]);
if ((data[6] & BIT(5)) && tmp)
@@ -1021,6 +1062,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
default:
+bail:
mcp->status = -EIO;
complete(&mcp->wait_in_report);
}
--
2.51.0
^ permalink raw reply related
* Re: [syzbot] [input?] [usb?] KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
From: syzbot @ 2025-10-15 12:08 UTC (permalink / raw)
To: bentiss, jikos, linux-input, linux-kernel, linux-usb, oneukum,
syzkaller-bugs
In-Reply-To: <f945f2a0-c694-4874-a7b2-59cee4cb76c3@suse.com>
Hello,
syzbot tried to test the proposed patch but the build/boot failed:
failed to checkout kernel repo git://repo/address.git on commit 3a8660878839: failed to run ["git" "fetch" "--force" "--tags" "b7cf8f2fbfc36c709a08e0b9c77990e491473738"]: exit status 128
Tested on:
commit: [unknown
git tree: git://repo/address.git 3a8660878839
kernel config: https://syzkaller.appspot.com/x/.config?x=f3e7b5a3627a90dd
dashboard link: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
compiler:
patch: https://syzkaller.appspot.com/x/patch.diff?x=13bed542580000
^ permalink raw reply
* Re: [syzbot] [input?] [usb?] KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
From: Oliver Neukum @ 2025-10-15 12:06 UTC (permalink / raw)
To: syzbot, bentiss, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <68ee6a3f.050a0220.91a22.021b.GAE@google.com>
[-- Attachment #1: Type: text/plain, Size: 14640 bytes --]
#syz test: git://repo/address.git 3a8660878839
On 14.10.25 17:20, syzbot wrote:
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit: 3a8660878839 Linux 6.18-rc1
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12a705e2580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f3e7b5a3627a90dd
> dashboard link: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
> compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=132ebb34580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=140fe52f980000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/e767e8931970/disk-3a866087.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/4cb12bdcfcea/vmlinux-3a866087.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/b08acfae954d/bzImage-3a866087.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+1018672fe70298606e5f@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in mcp2221_raw_event+0x1070/0x10a0 drivers/hid/hid-mcp2221.c:948
> Read of size 1 at addr ffff8880721cbfff by task kworker/0:7/6094
>
> CPU: 0 UID: 0 PID: 6094 Comm: kworker/0:7 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:378 [inline]
> print_report+0xcd/0x630 mm/kasan/report.c:482
> kasan_report+0xe0/0x110 mm/kasan/report.c:595
> mcp2221_raw_event+0x1070/0x10a0 drivers/hid/hid-mcp2221.c:948
> __hid_input_report.constprop.0+0x314/0x450 drivers/hid/hid-core.c:2139
> hid_irq_in+0x35e/0x870 drivers/hid/usbhid/hid-core.c:286
> __usb_hcd_giveback_urb+0x38b/0x610 drivers/usb/core/hcd.c:1661
> usb_hcd_giveback_urb+0x39b/0x450 drivers/usb/core/hcd.c:1745
> dummy_timer+0x1809/0x3a00 drivers/usb/gadget/udc/dummy_hcd.c:1995
> __run_hrtimer kernel/time/hrtimer.c:1777 [inline]
> __hrtimer_run_queues+0x202/0xad0 kernel/time/hrtimer.c:1841
> hrtimer_run_softirq+0x17d/0x350 kernel/time/hrtimer.c:1858
> handle_softirqs+0x219/0x8e0 kernel/softirq.c:622
> __do_softirq kernel/softirq.c:656 [inline]
> invoke_softirq kernel/softirq.c:496 [inline]
> __irq_exit_rcu+0x109/0x170 kernel/softirq.c:723
> irq_exit_rcu+0x9/0x30 kernel/softirq.c:739
> instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1052 [inline]
> sysvec_apic_timer_interrupt+0xa4/0xc0 arch/x86/kernel/apic/apic.c:1052
> </IRQ>
> <TASK>
> asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:697
> RIP: 0010:kasan_check_range+0x12/0x1b0 mm/kasan/generic.c:199
> Code: 00 00 00 00 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 85 f6 0f 84 64 01 00 00 48 89 f8 41 54 <44> 0f b6 c2 48 01 f0 55 53 0f 82 d7 00 00 00 eb 0f cc cc cc 48 b8
> RSP: 0018:ffffc900037b6b60 EFLAGS: 00000202
> RAX: ffff888077da86b0 RBX: ffff888077da8668 RCX: ffffffff819803ae
> RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888077da86b0
> RBP: ffff888077da86b0 R08: 0000000000000002 R09: 0000000000000000
> R10: ffff888077da866f R11: 0000000000000000 R12: ffffffff8c6df2a0
> R13: ffffffff9ae57620 R14: 0000000000000000 R15: ffff888026709978
> instrument_atomic_write include/linux/instrumented.h:82 [inline]
> atomic_set include/linux/atomic/atomic-instrumented.h:67 [inline]
> osq_lock_init include/linux/osq_lock.h:25 [inline]
> __mutex_init+0xae/0x120 kernel/locking/mutex.c:53
> i2c_register_adapter+0x15d/0x1370 drivers/i2c/i2c-core-base.c:1544
> i2c_add_adapter drivers/i2c/i2c-core-base.c:1673 [inline]
> i2c_add_adapter+0x10a/0x1b0 drivers/i2c/i2c-core-base.c:1653
> devm_i2c_add_adapter+0x1b/0x90 drivers/i2c/i2c-core-base.c:1845
> mcp2221_probe+0x5f1/0xc50 drivers/hid/hid-mcp2221.c:1289
> __hid_device_probe drivers/hid/hid-core.c:2775 [inline]
> hid_device_probe+0x5ba/0x8d0 drivers/hid/hid-core.c:2812
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> hid_add_device+0x31b/0x5c0 drivers/hid/hid-core.c:2951
> usbhid_probe+0xd38/0x13f0 drivers/hid/usbhid/hid-core.c:1435
> usb_probe_interface+0x303/0xa40 drivers/usb/core/driver.c:396
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> usb_set_configuration+0x1187/0x1e20 drivers/usb/core/message.c:2210
> usb_generic_driver_probe+0xb1/0x110 drivers/usb/core/generic.c:250
> usb_probe_device+0xef/0x3e0 drivers/usb/core/driver.c:291
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x241/0xa90 drivers/base/dd.c:659
> __driver_probe_device+0x1de/0x440 drivers/base/dd.c:801
> driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:831
> __device_attach_driver+0x1df/0x310 drivers/base/dd.c:959
> bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:462
> __device_attach+0x1e4/0x4b0 drivers/base/dd.c:1031
> bus_probe_device+0x17f/0x1c0 drivers/base/bus.c:537
> device_add+0x1148/0x1aa0 drivers/base/core.c:3689
> usb_new_device+0xd07/0x1a60 drivers/usb/core/hub.c:2694
> hub_port_connect drivers/usb/core/hub.c:5566 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
> port_event drivers/usb/core/hub.c:5870 [inline]
> hub_event+0x2f34/0x4fe0 drivers/usb/core/hub.c:5952
> process_one_work+0x9cf/0x1b70 kernel/workqueue.c:3263
> process_scheduled_works kernel/workqueue.c:3346 [inline]
> worker_thread+0x6c8/0xf10 kernel/workqueue.c:3427
> kthread+0x3c5/0x780 kernel/kthread.c:463
> ret_from_fork+0x675/0x7d0 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> </TASK>
>
> Allocated by task 5918:
> kasan_save_stack+0x33/0x60 mm/kasan/common.c:56
> kasan_save_track+0x14/0x30 mm/kasan/common.c:77
> poison_kmalloc_redzone mm/kasan/common.c:400 [inline]
> __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:417
> kmalloc_noprof include/linux/slab.h:957 [inline]
> kzalloc_noprof include/linux/slab.h:1094 [inline]
> ipv6_add_addr+0x4e3/0x1fe0 net/ipv6/addrconf.c:1120
> add_addr+0xde/0x350 net/ipv6/addrconf.c:3201
> add_v4_addrs+0x642/0x980 net/ipv6/addrconf.c:3263
> addrconf_gre_config net/ipv6/addrconf.c:3545 [inline]
> addrconf_init_auto_addrs+0x51a/0x810 net/ipv6/addrconf.c:3559
> addrconf_notify+0xe93/0x19e0 net/ipv6/addrconf.c:3740
> notifier_call_chain+0xbc/0x410 kernel/notifier.c:85
> call_netdevice_notifiers_info+0xbe/0x140 net/core/dev.c:2229
> call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
> call_netdevice_notifiers net/core/dev.c:2281 [inline]
> __dev_notify_flags+0x12c/0x2e0 net/core/dev.c:9676
> netif_change_flags+0x108/0x160 net/core/dev.c:9705
> do_setlink.constprop.0+0xb53/0x4380 net/core/rtnetlink.c:3151
> rtnl_changelink net/core/rtnetlink.c:3769 [inline]
> __rtnl_newlink net/core/rtnetlink.c:3928 [inline]
> rtnl_newlink+0x1446/0x2000 net/core/rtnetlink.c:4065
> rtnetlink_rcv_msg+0x95e/0xe90 net/core/rtnetlink.c:6954
> netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
> netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
> netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1346
> netlink_sendmsg+0x8c8/0xdd0 net/netlink/af_netlink.c:1896
> sock_sendmsg_nosec net/socket.c:727 [inline]
> __sock_sendmsg net/socket.c:742 [inline]
> __sys_sendto+0x4a3/0x520 net/socket.c:2244
> __do_sys_sendto net/socket.c:2251 [inline]
> __se_sys_sendto net/socket.c:2247 [inline]
> __x64_sys_sendto+0xe0/0x1c0 net/socket.c:2247
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> The buggy address belongs to the object at ffff8880721cbc00
> which belongs to the cache kmalloc-cg-512 of size 512
> The buggy address is located 583 bytes to the right of
> allocated 440-byte region [ffff8880721cbc00, ffff8880721cbdb8)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x721c8
> head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
> page_type: f5(slab)
> raw: 00fff00000000040 ffff88813ff30140 ffffea0001e68c00 dead000000000002
> raw: 0000000000000000 0000000000100010 00000000f5000000 0000000000000000
> head: 00fff00000000040 ffff88813ff30140 ffffea0001e68c00 dead000000000002
> head: 0000000000000000 0000000000100010 00000000f5000000 0000000000000000
> head: 00fff00000000002 ffffea0001c87201 00000000ffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000004
> page dumped because: kasan: bad access detected
> page_owner tracks the page as allocated
> page last allocated via order 2, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 5796, tgid 5796 (sshd-session), ts 52056965840, free_ts 15121629475
> set_page_owner include/linux/page_owner.h:32 [inline]
> post_alloc_hook+0x1c0/0x230 mm/page_alloc.c:1850
> prep_new_page mm/page_alloc.c:1858 [inline]
> get_page_from_freelist+0x10a3/0x3a30 mm/page_alloc.c:3884
> __alloc_frozen_pages_noprof+0x25f/0x2470 mm/page_alloc.c:5183
> alloc_pages_mpol+0x1fb/0x550 mm/mempolicy.c:2416
> alloc_slab_page mm/slub.c:3039 [inline]
> allocate_slab mm/slub.c:3212 [inline]
> new_slab+0x24a/0x360 mm/slub.c:3266
> ___slab_alloc+0xdc4/0x1ae0 mm/slub.c:4636
> __slab_alloc.constprop.0+0x63/0x110 mm/slub.c:4755
> __slab_alloc_node mm/slub.c:4831 [inline]
> slab_alloc_node mm/slub.c:5253 [inline]
> __do_kmalloc_node mm/slub.c:5626 [inline]
> __kmalloc_node_track_caller_noprof+0x4db/0x8a0 mm/slub.c:5736
> kmalloc_reserve+0xef/0x2c0 net/core/skbuff.c:601
> __alloc_skb+0x166/0x380 net/core/skbuff.c:670
> alloc_skb include/linux/skbuff.h:1383 [inline]
> alloc_skb_with_frags+0xe0/0x860 net/core/skbuff.c:6671
> sock_alloc_send_pskb+0x7f9/0x980 net/core/sock.c:2965
> unix_stream_sendmsg+0x39f/0x1340 net/unix/af_unix.c:2455
> sock_sendmsg_nosec net/socket.c:727 [inline]
> __sock_sendmsg net/socket.c:742 [inline]
> sock_write_iter+0x566/0x610 net/socket.c:1195
> new_sync_write fs/read_write.c:593 [inline]
> vfs_write+0x7d3/0x11d0 fs/read_write.c:686
> ksys_write+0x1f8/0x250 fs/read_write.c:738
> page last free pid 1 tgid 1 stack trace:
> reset_page_owner include/linux/page_owner.h:25 [inline]
> free_pages_prepare mm/page_alloc.c:1394 [inline]
> __free_frozen_pages+0x7df/0x1160 mm/page_alloc.c:2906
> __free_pages mm/page_alloc.c:5302 [inline]
> free_contig_range+0x183/0x4b0 mm/page_alloc.c:7146
> destroy_args+0xb69/0x12e0 mm/debug_vm_pgtable.c:958
> debug_vm_pgtable+0x1a32/0x3640 mm/debug_vm_pgtable.c:1345
> do_one_initcall+0x123/0x6e0 init/main.c:1283
> do_initcall_level init/main.c:1345 [inline]
> do_initcalls init/main.c:1361 [inline]
> do_basic_setup init/main.c:1380 [inline]
> kernel_init_freeable+0x5c8/0x920 init/main.c:1593
> kernel_init+0x1c/0x2b0 init/main.c:1483
> ret_from_fork+0x675/0x7d0 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>
> Memory state around the buggy address:
> ffff8880721cbe80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8880721cbf00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff8880721cbf80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff8880721cc000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8880721cc080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
> ----------------
> Code disassembly (best guess):
> 0: 00 00 add %al,(%rax)
> 2: 00 00 add %al,(%rax)
> 4: 0f 1f 40 00 nopl 0x0(%rax)
> 8: 90 nop
> 9: 90 nop
> a: 90 nop
> b: 90 nop
> c: 90 nop
> d: 90 nop
> e: 90 nop
> f: 90 nop
> 10: 90 nop
> 11: 90 nop
> 12: 90 nop
> 13: 90 nop
> 14: 90 nop
> 15: 90 nop
> 16: 90 nop
> 17: 90 nop
> 18: 0f 1f 40 d6 nopl -0x2a(%rax)
> 1c: 48 85 f6 test %rsi,%rsi
> 1f: 0f 84 64 01 00 00 je 0x189
> 25: 48 89 f8 mov %rdi,%rax
> 28: 41 54 push %r12
> * 2a: 44 0f b6 c2 movzbl %dl,%r8d <-- trapping instruction
> 2e: 48 01 f0 add %rsi,%rax
> 31: 55 push %rbp
> 32: 53 push %rbx
> 33: 0f 82 d7 00 00 00 jb 0x110
> 39: eb 0f jmp 0x4a
> 3b: cc int3
> 3c: cc int3
> 3d: cc int3
> 3e: 48 rex.W
> 3f: b8 .byte 0xb8
>
>
> ---
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
[-- Attachment #2: 0001-hid-mcp2221-validate-message-length.patch --]
[-- Type: text/x-patch, Size: 4406 bytes --]
From 41216a0385b9d2ff1f42a860109bba286fe9d28b Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 15 Oct 2025 13:49:05 +0200
Subject: [PATCH] hid-mcp2221: validate message length
The message passed to raw_event is of indeterminate length.
Check for length before accessing members.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/hid/hid-mcp2221.c | 44 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 33603b019f97..d5e9f7ef8ba8 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -849,12 +849,18 @@ static int mcp2221_raw_event(struct hid_device *hdev,
u8 *buf;
struct mcp2221 *mcp = hid_get_drvdata(hdev);
+ if (size <= 0)
+ goto bail;
+
switch (data[0]) {
case MCP2221_I2C_WR_DATA:
case MCP2221_I2C_WR_NO_STOP:
case MCP2221_I2C_RD_DATA:
case MCP2221_I2C_RD_RPT_START:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -866,6 +872,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_I2C_PARAM_OR_STATUS:
+ if (size < 4)
+ goto bail;
switch (data[1]) {
case MCP2221_SUCCESS:
if ((mcp->txbuf[3] == MCP2221_I2C_SET_SPEED) &&
@@ -873,6 +881,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
mcp->status = -EAGAIN;
break;
}
+ if (size < 21)
+ goto bail;
if (data[20] & MCP2221_I2C_MASK_ADDR_NACK) {
mcp->status = -ENXIO;
break;
@@ -889,12 +899,19 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_I2C_GET_DATA:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < 3)
+ goto bail;
if (data[2] == MCP2221_I2C_ADDR_NACK) {
mcp->status = -ENXIO;
break;
}
+ if (size < 4)
+ goto bail;
if (!mcp_get_i2c_eng_state(mcp, data, 2)
&& (data[3] == 0)) {
mcp->status = 0;
@@ -906,7 +923,9 @@ static int mcp2221_raw_event(struct hid_device *hdev,
}
if (data[2] == MCP2221_I2C_READ_COMPL ||
data[2] == MCP2221_I2C_READ_PARTIAL) {
- if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) {
+ if (!mcp->rxbuf ||
+ mcp->rxbuf_idx < 0 || data[3] > 60 ||
+ data[3] > size - 4 ) {
mcp->status = -EINVAL;
break;
}
@@ -925,8 +944,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GPIO_GET:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (mcp->gp_idx < size)
+ goto bail;
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
(data[mcp->gp_idx + 1] == MCP2221_ALT_F_NOT_GPIOD)) {
mcp->status = -ENOENT;
@@ -942,8 +966,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GPIO_SET:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < mcp->gp_idx)
+ goto bail;
if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
(data[mcp->gp_idx - 1] == MCP2221_ALT_F_NOT_GPIOV)) {
mcp->status = -ENOENT;
@@ -958,6 +987,9 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_SET_SRAM_SETTINGS:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -969,8 +1001,13 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_GET_SRAM_SETTINGS:
+ if (size < 2)
+ goto bail;
+
switch (data[1]) {
case MCP2221_SUCCESS:
+ if (size < 22 + 4)
+ goto bail;
memcpy(&mcp->mode, &data[22], 4);
#if IS_REACHABLE(CONFIG_IIO)
mcp->dac_value = data[6] & GENMASK(4, 0);
@@ -984,6 +1021,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
case MCP2221_READ_FLASH_DATA:
+ if (size < 2)
+ goto bail;
switch (data[1]) {
case MCP2221_SUCCESS:
mcp->status = 0;
@@ -997,6 +1036,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
#if IS_REACHABLE(CONFIG_IIO)
{
u8 tmp;
+ if (size < 8)
+ goto bail;
/* DAC scale value */
tmp = FIELD_GET(GENMASK(7, 6), data[6]);
if ((data[6] & BIT(5)) && tmp)
@@ -1021,6 +1062,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
default:
+bail:
mcp->status = -EIO;
complete(&mcp->wait_in_report);
}
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v6 3/7] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
From: Ilpo Järvinen @ 2025-10-15 11:59 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <20251013201535.6737-4-lkml@antheas.dev>
On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> Some devices, such as the Z13 have multiple AURA devices connected
> to them by USB. In addition, they might have a WMI interface for
> RGB. In Windows, Armoury Crate exposes a unified brightness slider
> for all of them, with 3 brightness levels.
>
> Therefore, to be synergistic in Linux, and support existing tooling
> such as UPower, allow adding listeners to the RGB device of the WMI
> interface. If WMI does not exist, lazy initialize the interface.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Tested-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/platform/x86/asus-wmi.c | 118 ++++++++++++++++++---
> include/linux/platform_data/x86/asus-wmi.h | 16 +++
> 2 files changed, 121 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index e72a2b5d158e..a2a7cd61fd59 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -258,6 +258,8 @@ struct asus_wmi {
> int tpd_led_wk;
> struct led_classdev kbd_led;
> int kbd_led_wk;
> + bool kbd_led_avail;
> + bool kbd_led_registered;
> struct led_classdev lightbar_led;
> int lightbar_led_wk;
> struct led_classdev micmute_led;
> @@ -1530,6 +1532,53 @@ static void asus_wmi_battery_exit(struct asus_wmi *asus)
>
> /* LEDs ***********************************************************************/
>
> +struct asus_hid_ref {
> + struct list_head listeners;
> + struct asus_wmi *asus;
> + spinlock_t lock;
Please always document what a lock protects.
I started wonder why it needs to be spinlock?
It would seem rwsem is more natural for it as write is only needed at
probe/remove time (if there's no good reason for using a spinlock).
You're also missing include.
> +};
> +
> +struct asus_hid_ref asus_ref = {
Should be static ?
> + .listeners = LIST_HEAD_INIT(asus_ref.listeners),
> + .asus = NULL,
> + .lock = __SPIN_LOCK_UNLOCKED(asus_ref.lock),
> +};
> +
> +int asus_hid_register_listener(struct asus_hid_listener *bdev)
> +{
> + unsigned long flags;
> + int ret = 0;
> +
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + list_add_tail(&bdev->list, &asus_ref.listeners);
> + if (asus_ref.asus) {
> + if (asus_ref.asus->kbd_led_registered && asus_ref.asus->kbd_led_wk >= 0)
> + bdev->brightness_set(bdev, asus_ref.asus->kbd_led_wk);
> +
> + if (!asus_ref.asus->kbd_led_registered) {
> + ret = led_classdev_register(
> + &asus_ref.asus->platform_device->dev,
> + &asus_ref.asus->kbd_led);
> + if (!ret)
> + asus_ref.asus->kbd_led_registered = true;
I suggest you use guard() for the spinlock and return early where possible.
With guard() in use, you can do normal error handling here with return ret
immediately.
Please also add a local var for asus_ref.asus.
> + }
> + }
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_register_listener);
> +
> +void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + list_del(&bdev->list);
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_unregister_listener);
> +
> /*
> * These functions actually update the LED's, and are called from a
> * workqueue. By doing this as separate work rather than when the LED
> @@ -1609,6 +1658,7 @@ static int kbd_led_read(struct asus_wmi *asus, int *level, int *env)
>
> static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> {
> + struct asus_hid_listener *listener;
> struct asus_wmi *asus;
> int max_level;
>
> @@ -1616,25 +1666,39 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> max_level = asus->kbd_led.max_brightness;
>
> asus->kbd_led_wk = clamp_val(value, 0, max_level);
> - kbd_led_update(asus);
> +
> + if (asus->kbd_led_avail)
> + kbd_led_update(asus);
> +
> + list_for_each_entry(listener, &asus_ref.listeners, list)
> + listener->brightness_set(listener, asus->kbd_led_wk);
> }
>
> static void kbd_led_set(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> + unsigned long flags;
> +
> /* Prevent disabling keyboard backlight on module unregister */
> if (led_cdev->flags & LED_UNREGISTERING)
> return;
>
> + spin_lock_irqsave(&asus_ref.lock, flags);
> do_kbd_led_set(led_cdev, value);
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> }
>
> static void kbd_led_set_by_kbd(struct asus_wmi *asus, enum led_brightness value)
> {
> - struct led_classdev *led_cdev = &asus->kbd_led;
> + struct led_classdev *led_cdev;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + led_cdev = &asus->kbd_led;
>
I'd remove the empty line from the critical section.
I think you should mention the locking in the changelog too as it clearly
impacts not only the new code.
> do_kbd_led_set(led_cdev, value);
> led_classdev_notify_brightness_hw_changed(led_cdev, asus->kbd_led_wk);
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> }
>
> static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
> @@ -1644,6 +1708,9 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
>
> asus = container_of(led_cdev, struct asus_wmi, kbd_led);
>
> + if (!asus->kbd_led_avail)
> + return asus->kbd_led_wk;
> +
> retval = kbd_led_read(asus, &value, NULL);
> if (retval < 0)
> return retval;
> @@ -1759,7 +1826,15 @@ static int camera_led_set(struct led_classdev *led_cdev,
>
> static void asus_wmi_led_exit(struct asus_wmi *asus)
> {
> - led_classdev_unregister(&asus->kbd_led);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + asus_ref.asus = NULL;
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
> +
> + if (asus->kbd_led_registered)
> + led_classdev_unregister(&asus->kbd_led);
> +
> led_classdev_unregister(&asus->tpd_led);
> led_classdev_unregister(&asus->wlan_led);
> led_classdev_unregister(&asus->lightbar_led);
> @@ -1773,6 +1848,8 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
> static int asus_wmi_led_init(struct asus_wmi *asus)
> {
> int rv = 0, num_rgb_groups = 0, led_val;
> + struct asus_hid_listener *listener;
> + unsigned long flags;
>
> if (asus->kbd_rgb_dev)
> kbd_rgb_mode_groups[num_rgb_groups++] = &kbd_rgb_mode_group;
> @@ -1797,23 +1874,38 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> goto error;
> }
>
> - if (!kbd_led_read(asus, &led_val, NULL) && !dmi_check_system(asus_use_hid_led_dmi_ids)) {
> - pr_info("using asus-wmi for asus::kbd_backlight\n");
> + asus->kbd_led.name = "asus::kbd_backlight";
> + asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> + asus->kbd_led.brightness_set = kbd_led_set;
> + asus->kbd_led.brightness_get = kbd_led_get;
> + asus->kbd_led.max_brightness = 3;
> + asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> +
> + if (asus->kbd_led_avail)
> asus->kbd_led_wk = led_val;
> - asus->kbd_led.name = "asus::kbd_backlight";
> - asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> - asus->kbd_led.brightness_set = kbd_led_set;
> - asus->kbd_led.brightness_get = kbd_led_get;
> - asus->kbd_led.max_brightness = 3;
> + else
> + asus->kbd_led_wk = -1;
>
> - if (num_rgb_groups != 0)
> - asus->kbd_led.groups = kbd_rgb_mode_groups;
> + if (asus->kbd_led_avail && num_rgb_groups != 0)
> + asus->kbd_led.groups = kbd_rgb_mode_groups;
Can't this be placed into the block above?
>
> + spin_lock_irqsave(&asus_ref.lock, flags);
> + if (asus->kbd_led_avail || !list_empty(&asus_ref.listeners)) {
> rv = led_classdev_register(&asus->platform_device->dev,
> &asus->kbd_led);
> - if (rv)
> + if (rv) {
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
Please use scoped_guard() so you don't need to call unlock yourself.
Unrelated to this patch, I'd also simplify error label by adding return 0
before it so the code after the label doesn't need if (rv) check.
> goto error;
> + }
> + asus->kbd_led_registered = true;
> +
> + if (asus->kbd_led_wk >= 0) {
> + list_for_each_entry(listener, &asus_ref.listeners, list)
> + listener->brightness_set(listener, asus->kbd_led_wk);
> + }
> }
> + asus_ref.asus = asus;
> + spin_unlock_irqrestore(&asus_ref.lock, flags);
>
> if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_WIRELESS_LED)
> && (asus->driver->quirks->wapf > 0)) {
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 8a515179113d..f13a701f47a8 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -163,11 +163,19 @@ enum asus_ally_mcu_hack {
> ASUS_WMI_ALLY_MCU_HACK_DISABLED,
> };
>
> +struct asus_hid_listener {
> + struct list_head list;
> + void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> +};
Please add kerneldoc to this struct and the exported functions.
> +
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> void set_ally_mcu_powersave(bool enabled);
> int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> +
> +int asus_hid_register_listener(struct asus_hid_listener *cdev);
> +void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> #else
> static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> {
> @@ -184,6 +192,14 @@ static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
> {
> return -ENODEV;
> }
> +
> +static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> +{
> + return -ENODEV;
> +}
> +static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> +{
> +}
> #endif
>
> /* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
>
--
i.
^ permalink raw reply
* Re: [PATCH v6 1/7] HID: asus: refactor init sequence per spec
From: Ilpo Järvinen @ 2025-10-15 11:30 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <CAGwozwF+LfL1AhR3PLJWLzF1iriohWFJRmRkHC6uwgfTnhZFaw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8734 bytes --]
On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
> On Wed, 15 Oct 2025 at 12:53, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> >
> > > Currently, asus_kbd_init() uses a reverse engineered init sequence
> > > from Windows, which contains the handshakes from multiple programs.
> > > Keep the main one, which is 0x5a (meant for brightness drivers).
> > >
> > > In addition, perform a get_response and check if the response is the
> > > same. To avoid regressions, print an error if the response does not
> > > match instead of rejecting device.
> >
> > I'm none the wiser on "why?" question after reading all this. Please
> > describe the change properly. **Besides, you do many thing changes which are
> > not mentioned here at all.**
>
> Changes in asus_kbd_register_leds look bigger than they are due to
> un-indenting and merging the if/else for non-nkey/nkey.
>
> I will update the text of the new patch to include the changes which
> are 1) applying asus_kbd_get_functions to NKEY devices to check for
> backlight, 2) removing 0x5d/0x5e initialization from NKEY devices
> (0x5d is for armoury crate/0x5e for an aura matrix creator studio
> thing), which then means that the if/else blocks are equivalent and
> can be merged.
Oh, I see it now. It was not at all obvious you wanted to consolidate the
init path because of all the other moving parts, to me it looked like you
just removed the if (<quirk>) check.
> These two changes should not affect functionality, other than reduce
> some init commands.
>
> > And what "spec" is the one you mention in the shortlog?
> >
> > > Then, refactor asus_kbd_get_functions() to use the same ID it is called
> > > with, instead of hardcoding it to 0x5a so that it may be used for 0x0d
> > > in the future.
> >
> > Can this be in own patch?
>
> I will spin this into three patches and reword. One for each paragraph
> in the current commit body.
3 patches already sounds much better! They'll surely be easier to review
and understand if somebody has to look at the commits years later from the
commit history.
Thanks.
--
i.
> Ack on the rest.
>
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > > drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++--------------------
> > > 1 file changed, 48 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > index a444d41e53b6..d0c783df99bc 100644
> > > --- a/drivers/hid/hid-asus.c
> > > +++ b/drivers/hid/hid-asus.c
> > > @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > > #define FEATURE_REPORT_ID 0x0d
> > > #define INPUT_REPORT_ID 0x5d
> > > #define FEATURE_KBD_REPORT_ID 0x5a
> > > -#define FEATURE_KBD_REPORT_SIZE 16
> > > +#define FEATURE_KBD_REPORT_SIZE 64
> > > #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> > > #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> > >
> > > @@ -393,14 +393,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
> > >
> > > static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> > > {
> > > - const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> > > - 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > > + /*
> > > + * The handshake is first sent as a set_report, then retrieved
> > > + * from a get_report. They should be equal.
> > > + */
> > > + const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20,
> > > + 0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> >
> > Why was layout of this changed?
> >
> > > + u8 *readbuf;
> > > int ret;
> > >
> > > ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> > > - if (ret < 0)
> > > - hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> > > + if (ret < 0) {
> > > + hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> > > + if (!readbuf)
> > > + return -ENOMEM;
> > > +
> > > + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> > > + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> > > + HID_REQ_GET_REPORT);
> > > + if (ret < 0) {
> > > + hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> > > + } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> > > + hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> > > + FEATURE_KBD_REPORT_SIZE, readbuf);
> > > + // Do not return error if handshake is wrong to avoid regressions
> >
> > This driver so far is using only /* */ comments.
> >
> > > + }
> > >
> > > + kfree(readbuf);
> > > return ret;
> > > }
> > >
> > > @@ -422,7 +445,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
> > > if (!readbuf)
> > > return -ENOMEM;
> > >
> > > - ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
> > > + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> > > FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> > > HID_REQ_GET_REPORT);
> > > if (ret < 0) {
> > > @@ -638,50 +661,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > > unsigned char kbd_func;
> > > int ret;
> > >
> > > - if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> > > - /* Initialize keyboard */
> > > - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > > - if (ret < 0)
> > > - return ret;
> > > -
> > > - /* The LED endpoint is initialised in two HID */
> > > - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> > > - if (ret < 0)
> > > - return ret;
> > > -
> > > - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> > > - if (ret < 0)
> > > - return ret;
> > > -
> > > - if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > > - ret = asus_kbd_disable_oobe(hdev);
> > > - if (ret < 0)
> > > - return ret;
> > > - }
> > > -
> > > - if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > > - intf = to_usb_interface(hdev->dev.parent);
> > > - udev = interface_to_usbdev(intf);
> > > - validate_mcu_fw_version(hdev,
> > > - le16_to_cpu(udev->descriptor.idProduct));
> > > - }
> > > + ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > > + if (ret < 0)
> > > + return ret;
> > >
> > > - } else {
> > > - /* Initialize keyboard */
> > > - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > > - if (ret < 0)
> > > - return ret;
> > > + /* Get keyboard functions */
> > > + ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > > + if (ret < 0)
> > > + return ret;
> > >
> > > - /* Get keyboard functions */
> > > - ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > > + if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > > + ret = asus_kbd_disable_oobe(hdev);
> > > if (ret < 0)
> > > return ret;
> > > + }
> > >
> > > - /* Check for backlight support */
> > > - if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > > - return -ENODEV;
> > > + if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > > + intf = to_usb_interface(hdev->dev.parent);
> > > + udev = interface_to_usbdev(intf);
> > > + validate_mcu_fw_version(
> > > + hdev, le16_to_cpu(udev->descriptor.idProduct));
> > > }
> > >
> > > + /* Check for backlight support */
> > > + if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > > + return -ENODEV;
> > > +
> > > drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> > > sizeof(struct asus_kbd_leds),
> > > GFP_KERNEL);
> > >
> >
> > --
> > i.
> >
> >
>
^ permalink raw reply
* Re: [PATCH v6 1/7] HID: asus: refactor init sequence per spec
From: Antheas Kapenekakis @ 2025-10-15 11:18 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <3d59f42f-2e24-6011-23b5-369be7eb4b3b@linux.intel.com>
On Wed, 15 Oct 2025 at 12:53, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
>
> > Currently, asus_kbd_init() uses a reverse engineered init sequence
> > from Windows, which contains the handshakes from multiple programs.
> > Keep the main one, which is 0x5a (meant for brightness drivers).
> >
> > In addition, perform a get_response and check if the response is the
> > same. To avoid regressions, print an error if the response does not
> > match instead of rejecting device.
>
> I'm none the wiser on "why?" question after reading all this. Please
> describe the change properly. **Besides, you do many thing changes which are
> not mentioned here at all.**
Changes in asus_kbd_register_leds look bigger than they are due to
un-indenting and merging the if/else for non-nkey/nkey.
I will update the text of the new patch to include the changes which
are 1) applying asus_kbd_get_functions to NKEY devices to check for
backlight, 2) removing 0x5d/0x5e initialization from NKEY devices
(0x5d is for armoury crate/0x5e for an aura matrix creator studio
thing), which then means that the if/else blocks are equivalent and
can be merged.
These two changes should not affect functionality, other than reduce
some init commands.
> And what "spec" is the one you mention in the shortlog?
>
> > Then, refactor asus_kbd_get_functions() to use the same ID it is called
> > with, instead of hardcoding it to 0x5a so that it may be used for 0x0d
> > in the future.
>
> Can this be in own patch?
I will spin this into three patches and reword. One for each paragraph
in the current commit body.
Ack on the rest.
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> > drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++--------------------
> > 1 file changed, 48 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index a444d41e53b6..d0c783df99bc 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> > @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > #define FEATURE_REPORT_ID 0x0d
> > #define INPUT_REPORT_ID 0x5d
> > #define FEATURE_KBD_REPORT_ID 0x5a
> > -#define FEATURE_KBD_REPORT_SIZE 16
> > +#define FEATURE_KBD_REPORT_SIZE 64
> > #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> > #define FEATURE_KBD_LED_REPORT_ID2 0x5e
> >
> > @@ -393,14 +393,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
> >
> > static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> > {
> > - const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> > - 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> > + /*
> > + * The handshake is first sent as a set_report, then retrieved
> > + * from a get_report. They should be equal.
> > + */
> > + const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20,
> > + 0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
>
> Why was layout of this changed?
>
> > + u8 *readbuf;
> > int ret;
> >
> > ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> > - if (ret < 0)
> > - hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> > + if (ret < 0) {
> > + hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> > + if (!readbuf)
> > + return -ENOMEM;
> > +
> > + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> > + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> > + HID_REQ_GET_REPORT);
> > + if (ret < 0) {
> > + hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> > + } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> > + hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> > + FEATURE_KBD_REPORT_SIZE, readbuf);
> > + // Do not return error if handshake is wrong to avoid regressions
>
> This driver so far is using only /* */ comments.
>
> > + }
> >
> > + kfree(readbuf);
> > return ret;
> > }
> >
> > @@ -422,7 +445,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
> > if (!readbuf)
> > return -ENOMEM;
> >
> > - ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
> > + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> > FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> > HID_REQ_GET_REPORT);
> > if (ret < 0) {
> > @@ -638,50 +661,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> > unsigned char kbd_func;
> > int ret;
> >
> > - if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> > - /* Initialize keyboard */
> > - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > - if (ret < 0)
> > - return ret;
> > -
> > - /* The LED endpoint is initialised in two HID */
> > - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> > - if (ret < 0)
> > - return ret;
> > -
> > - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> > - if (ret < 0)
> > - return ret;
> > -
> > - if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > - ret = asus_kbd_disable_oobe(hdev);
> > - if (ret < 0)
> > - return ret;
> > - }
> > -
> > - if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > - intf = to_usb_interface(hdev->dev.parent);
> > - udev = interface_to_usbdev(intf);
> > - validate_mcu_fw_version(hdev,
> > - le16_to_cpu(udev->descriptor.idProduct));
> > - }
> > + ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > + if (ret < 0)
> > + return ret;
> >
> > - } else {
> > - /* Initialize keyboard */
> > - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> > - if (ret < 0)
> > - return ret;
> > + /* Get keyboard functions */
> > + ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > + if (ret < 0)
> > + return ret;
> >
> > - /* Get keyboard functions */
> > - ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> > + if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> > + ret = asus_kbd_disable_oobe(hdev);
> > if (ret < 0)
> > return ret;
> > + }
> >
> > - /* Check for backlight support */
> > - if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > - return -ENODEV;
> > + if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> > + intf = to_usb_interface(hdev->dev.parent);
> > + udev = interface_to_usbdev(intf);
> > + validate_mcu_fw_version(
> > + hdev, le16_to_cpu(udev->descriptor.idProduct));
> > }
> >
> > + /* Check for backlight support */
> > + if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> > + return -ENODEV;
> > +
> > drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> > sizeof(struct asus_kbd_leds),
> > GFP_KERNEL);
> >
>
> --
> i.
>
>
^ permalink raw reply
* Re: [PATCH v6 2/7] HID: asus: prevent binding to all HID devices on ROG
From: Ilpo Järvinen @ 2025-10-15 10:59 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <20251013201535.6737-3-lkml@antheas.dev>
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@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.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.
^ permalink raw reply
* Re: [PATCH v6 1/7] HID: asus: refactor init sequence per spec
From: Ilpo Järvinen @ 2025-10-15 10:53 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <20251013201535.6737-2-lkml@antheas.dev>
On Mon, 13 Oct 2025, Antheas Kapenekakis wrote:
> Currently, asus_kbd_init() uses a reverse engineered init sequence
> from Windows, which contains the handshakes from multiple programs.
> Keep the main one, which is 0x5a (meant for brightness drivers).
>
> In addition, perform a get_response and check if the response is the
> same. To avoid regressions, print an error if the response does not
> match instead of rejecting device.
I'm none the wiser on "why?" question after reading all this. Please
describe the change properly. Besides, you do many thing changes which are
not mentioned here at all.
And what "spec" is the one you mention in the shortlog?
> Then, refactor asus_kbd_get_functions() to use the same ID it is called
> with, instead of hardcoding it to 0x5a so that it may be used for 0x0d
> in the future.
Can this be in own patch?
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++--------------------
> 1 file changed, 48 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a444d41e53b6..d0c783df99bc 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -48,7 +48,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define FEATURE_REPORT_ID 0x0d
> #define INPUT_REPORT_ID 0x5d
> #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>
> @@ -393,14 +393,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>
> static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> {
> - const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> - 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> + /*
> + * The handshake is first sent as a set_report, then retrieved
> + * from a get_report. They should be equal.
> + */
> + const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20,
> + 0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
Why was layout of this changed?
> + u8 *readbuf;
> int ret;
>
> ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> - if (ret < 0)
> - hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> + return ret;
> + }
> +
> + readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> + if (!readbuf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> + } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> + hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> + FEATURE_KBD_REPORT_SIZE, readbuf);
> + // Do not return error if handshake is wrong to avoid regressions
This driver so far is using only /* */ comments.
> + }
>
> + kfree(readbuf);
> return ret;
> }
>
> @@ -422,7 +445,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
> if (!readbuf)
> return -ENOMEM;
>
> - ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
> + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> HID_REQ_GET_REPORT);
> if (ret < 0) {
> @@ -638,50 +661,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> unsigned char kbd_func;
> int ret;
>
> - if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> - /* Initialize keyboard */
> - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> - if (ret < 0)
> - return ret;
> -
> - /* The LED endpoint is initialised in two HID */
> - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> - if (ret < 0)
> - return ret;
> -
> - ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> - if (ret < 0)
> - return ret;
> -
> - if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> - ret = asus_kbd_disable_oobe(hdev);
> - if (ret < 0)
> - return ret;
> - }
> -
> - if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> - intf = to_usb_interface(hdev->dev.parent);
> - udev = interface_to_usbdev(intf);
> - validate_mcu_fw_version(hdev,
> - le16_to_cpu(udev->descriptor.idProduct));
> - }
> + ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> + if (ret < 0)
> + return ret;
>
> - } else {
> - /* Initialize keyboard */
> - ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> - if (ret < 0)
> - return ret;
> + /* Get keyboard functions */
> + ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> + if (ret < 0)
> + return ret;
>
> - /* Get keyboard functions */
> - ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> + if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> + ret = asus_kbd_disable_oobe(hdev);
> if (ret < 0)
> return ret;
> + }
>
> - /* Check for backlight support */
> - if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> - return -ENODEV;
> + if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> + intf = to_usb_interface(hdev->dev.parent);
> + udev = interface_to_usbdev(intf);
> + validate_mcu_fw_version(
> + hdev, le16_to_cpu(udev->descriptor.idProduct));
> }
>
> + /* Check for backlight support */
> + if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> + return -ENODEV;
> +
> drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> sizeof(struct asus_kbd_leds),
> GFP_KERNEL);
>
--
i.
^ permalink raw reply
* 答复: [External Mail]Re: [PATCH] HID: hid-input: only ignore 0 battery events for digitizers
From: 卢国宏 @ 2025-10-15 7:48 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina
Cc: Benjamin Tissoires, kenalba@google.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
李鹏, 卢国宏
In-Reply-To: <nsz6llo4nsg4zuiogmewuccl76nzjjijgkjzqeazzs33lj4phc@k5rskuabbsz2>
[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]
Hi Jiri,
The last time I verified Dmitry's patch, it was based on the latest Linux code. The patch at that time is attached as "Patch reporting HID device zero battery.jpg". As I understand it, you can submit Dmitry's V2 patch directly to the kernel without retesting.
Thanks.
--
guohong
________________________________________
发件人: Dmitry Torokhov <dmitry.torokhov@gmail.com>
发送时间: 2025年10月15日 12:31:10
收件人: Jiri Kosina
抄送: Benjamin Tissoires; 卢国宏; kenalba@google.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
主题: [External Mail]Re: [PATCH] HID: hid-input: only ignore 0 battery events for digitizers
[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
Hi Jiri,
On Tue, Oct 14, 2025 at 12:25:25PM +0200, Jiri Kosina wrote:
>
> Hi Dmitry,
>
> thanks for the fix. It doesn't apply cleanly on a reasonably recent tree
> though, as since e94536e1d1818b09 we're already propagating usage to
> hidinput_update_battery(), and we're issuing explicit call to
> power_supply_changed() as well.
>
> Could you please refresh on a more recent codebase and resubmit? I could
> do that myself, but I guess you also have a way to test the patch with
> your use-case ... ?
Sorry about that, I made the patch on top of my tree which lags behind.
I just sent a v2 rebased on top of linux-next.
I actually do not have the hardware to test, we have to rely on 卢国宏
to do it for us.
Thanks.
--
Dmitry
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
[-- Attachment #2: Patch reporting HID device zero battery.jpg --]
[-- Type: image/jpeg, Size: 482355 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox