| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9be2c01e-190b-4d55-8ff8-3d89be52fc84@ljones.dev>
Date: Sat, 22 Mar 2025 17:31:19 +1300
From: "Luke D. Jones" <luke@...nes.dev>
To: Antheas Kapenekakis <lkml@...heas.dev>,
platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>,
Corentin Chary <corentin.chary@...il.com>,
Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH 08/11] platform/x86: asus-wmi: add keyboard brightness
event handler
On 21/03/25 11:09, Antheas Kapenekakis wrote:
> Currenlty, the keyboard brightness control of Asus WMI keyboards is
> 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.
>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
> drivers/platform/x86/asus-wmi.c | 38 ++++++++++++++++++++++
> include/linux/platform_data/x86/asus-wmi.h | 11 +++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 21e034be71b2f..45999dda9e7ed 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1529,6 +1529,44 @@ void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
> }
> EXPORT_SYMBOL_GPL(asus_brt_unregister_listener);
>
> +static void do_kbd_led_set(struct led_classdev *led_cdev, int value);
> +
> +int asus_brt_event(enum asus_brt_event event)
> +{
> + int brightness;
> +
> + mutex_lock(&asus_brt_lock);
> + if (!asus_brt_ref || !asus_brt_ref->kbd_led_registered) {
> + mutex_unlock(&asus_brt_lock);
> + return -EBUSY;
> + }
> + brightness = asus_brt_ref->kbd_led_wk;
> +
> + switch (event) {
> + case ASUS_BRT_UP:
> + brightness += 1;
> + break;
> + case ASUS_BRT_DOWN:
> + brightness -= 1;
> + break;
> + case ASUS_BRT_TOGGLE:
> + if (brightness >= 3)
> + brightness = 0;
> + else
> + brightness += 1;
> + break;
> + }
> +
> + do_kbd_led_set(&asus_brt_ref->kbd_led, brightness);
> + led_classdev_notify_brightness_hw_changed(&asus_brt_ref->kbd_led,
> + asus_brt_ref->kbd_led_wk);
> +
> + mutex_unlock(&asus_brt_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_brt_event);
> +
I quick test on 6.14-rc7 gives me this:
[ 288.039255] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:258
[ 288.039262] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0,
name: swapper/17
[ 288.039263] preempt_count: 101, expected: 0
[ 288.039264] RCU nest depth: 0, expected: 0
[ 288.039266] CPU: 17 UID: 0 PID: 0 Comm: swapper/17 Tainted: G
W 6.14.0-rc7+ #164
[ 288.039268] Tainted: [W]=WARN
[ 288.039269] Hardware name: ASUSTeK COMPUTER INC. ROG Zephyrus M16
GU604VY_GU604VY_00130747B/GU604VY, BIOS GU604VY.313 08/13/2024
[ 288.039270] Call Trace:
[ 288.039272] <IRQ>
[ 288.039273] dump_stack_lvl+0x5d/0x80
[ 288.039277] __might_resched.cold+0xba/0xc9
[ 288.039282] mutex_lock+0x19/0x40
[ 288.039285] asus_brt_event+0x13/0xb0 [asus_wmi]
[ 288.039292] asus_event+0x91/0xa0 [hid_asus]
[ 288.039295] hid_process_event+0xae/0x120
[ 288.039298] hid_input_array_field+0x132/0x180
[ 288.039300] hid_report_raw_event+0x360/0x4e0
[ 288.039302] __hid_input_report.constprop.0+0xf1/0x180
[ 288.039304] hid_irq_in+0x17f/0x1b0
[ 288.039306] __usb_hcd_giveback_urb+0x98/0x110
[ 288.039308] usb_giveback_urb_bh+0xbd/0x150
[ 288.039310] process_one_work+0x171/0x290
[ 288.039312] bh_worker+0x1ac/0x210
[ 288.039314] tasklet_hi_action+0xe/0x30
[ 288.039315] handle_softirqs+0xdb/0x1f0
[ 288.039317] __irq_exit_rcu+0xc2/0xe0
[ 288.039318] common_interrupt+0x85/0xa0
[ 288.039320] </IRQ>
[ 288.039320] <TASK>
[ 288.039321] asm_common_interrupt+0x26/0x40
[ 288.039323] RIP: 0010:cpuidle_enter_state+0xb9/0x2c0
[ 288.039325] Code: 40 0f 84 1b 01 00 00 e8 35 e8 13 ff e8 40 f2 ff ff
31 ff 49 89 c5 e8 c6 f0 12 ff 45 84 f6 0f 85 f2 00 00 00 fb 0f 1f 44 00
00 <45> 85 ff 0f 88 cf 00 00 00 49 63 f7 48 8d 04 76 48 8d 04 86 49 8d
[ 288.039326] RSP: 0018:ffffc90000253e90 EFLAGS: 00000246
[ 288.039328] RAX: ffff888890680000 RBX: 0000000000000003 RCX:
0000000000000000
[ 288.039329] RDX: 00000043107862af RSI: fffffffd616e8210 RDI:
0000000000000000
[ 288.039329] RBP: ffff8888906ba370 R08: 0000000000000000 R09:
0000000000000007
[ 288.039330] R10: ffff88888ffa6098 R11: 0000000000000008 R12:
ffffffff82fd4140
[ 288.039331] R13: 00000043107862af R14: 0000000000000000 R15:
0000000000000003
[ 288.039332] cpuidle_enter+0x28/0x40
[ 288.039334] do_idle+0x1a8/0x200
[ 288.039336] cpu_startup_entry+0x24/0x30
[ 288.039337] start_secondary+0x11e/0x140
[ 288.039340] common_startup_64+0x13e/0x141
[ 288.039342] </TASK>
I think you need to swap the mutex to a spin_lock_irqsave and associated
spin_unlock_irqrestore plus DEFINE_SPINLOCK(asus_brt_lock).
I'm out of time tonight but I'll apply the above to your patches and
report back tomorrow if you don't get to it before I do.
It might be worth checking any other mutex uses in the LED path too.
Cheers,
Luke.
> /*
> * These functions actually update the LED's, and are called from a
> * workqueue. By doing this as separate work rather than when the LED
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index add04524031d8..2ac7912d8acd3 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -162,11 +162,18 @@ struct asus_brt_listener {
> void (*notify)(struct asus_brt_listener *listener, int brightness);
> };
>
> +enum asus_brt_event {
> + ASUS_BRT_UP,
> + ASUS_BRT_DOWN,
> + ASUS_BRT_TOGGLE,
> +};
> +
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
>
> int asus_brt_register_listener(struct asus_brt_listener *cdev);
> void asus_brt_unregister_listener(struct asus_brt_listener *cdev);
> +int asus_brt_event(enum asus_brt_event event);
> #else
> static inline int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
> u32 *retval)
> @@ -181,6 +188,10 @@ static inline int asus_brt_register_listener(struct asus_brt_listener *bdev)
> static inline void asus_brt_unregister_listener(struct asus_brt_listener *bdev)
> {
> }
> +static inline int asus_brt_event(enum asus_brt_event event)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
Powered by blists - more mailing lists