linux-kernel - Re: [PATCH v6] tty: Fix the keyboard led light display problem

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <YXY8zrEbkhdO6ikS@google.com>
Date:   Sun, 24 Oct 2021 22:12:46 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     lianzhi chang <changlianzhi@...ontech.com>
Cc:     linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
        jirislaby@...nel.org, andriy.shevchenko@...ux.intel.com,
        282827961@...com
Subject: Re: [PATCH v6] tty: Fix the keyboard led light display problem

Hi,

On Thu, Oct 21, 2021 at 10:05:11AM +0800, lianzhi chang wrote:
> @@ -1524,6 +1543,9 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
>  	/* We are called with interrupts disabled, just take the lock */
>  	spin_lock(&kbd_event_lock);
>  
> +	if (test_bit(EV_LED, handle->dev->evbit))
> +		kbd_update_ledstate(handle->dev);
> +

Why do we need to update led state on each and every event instead of
trapping individual EV_LED/LED_x events?

But thinking about it some more, I believe doing this here is wrong.
LEDs are routed in this way:

   device <-> input core <-> input leds <-> leds core <-> VT

with the last step assuming that we are in default configuration and we
indeed assign one of the following triggers to appropriate LED on given
device:

kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock
kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock
kbd-ctrlllock kbd-ctrlrlock

I.e. the state of LED of individual input device is not necessarily
attached to LED flag state in a given VT.

Re-reading the patch description, are we saying that we are maintaining
kbd->[s]lockstate, but losing kbd->ledflagstate when we are doing
transitions to and from VC_OFF state? We need to keep in mind that we
are trying to keep states separate on separate consoles.

Thanks.

-- 
Dmitry