linux-kernel - Re: [PATCH v18] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YaS7BdPTkgPyw1/e@smile.fi.intel.com>
Date:   Mon, 29 Nov 2021 13:35:33 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     lianzhi chang <changlianzhi@...ontech.com>
Cc:     linux-kernel@...r.kernel.org, dmitry.torokhov@...il.com,
        gregkh@...uxfoundation.org, jirislaby@...nel.org, 282827961@...com
Subject: Re: [PATCH v18] tty: Fix the keyboard led light display problem

On Mon, Nov 29, 2021 at 12:04:45PM +0800, lianzhi chang wrote:
> Use the "ctrl+alt+Fn" key combination to switch the system from tty to
> desktop or switch the system from desktop to tty. After the switch is
> completed, it is found that the state of the keyboard lock is
> inconsistent with the state of the keyboard Led light.The reasons are
> as follows:
> 
> * The desktop environment (Xorg and other services) is bound to a tty
>   (assuming it is tty1), and the kb->kbdmode attribute value of tty1
>   will be set to VC_OFF. According to the current code logic, in the
>   desktop environment, the values of ledstate and kb->ledflagstate
>   of tty1 will not be modified anymore, so they are always 0.
> 
> * When switching between each tty, the final value of ledstate set by
>   the previous tty is compared with the kb->ledflagstate value of the
>   current tty to determine whether to set the state of the keyboard
>   light. The process of switching between desktop and tty is also the
>   process of switching between tty1 and other ttys. There are two
>   situations:
> 
>   - (1) In the desktop environment, tty1 will not set the ledstate,
>   which will cause when switching from the desktop to other ttys,
>   if the desktop lights up the keyboard's led, after the switch is
>   completed, the keyboard's led light will always be on;
> 
>   - (2) When switching from another tty to the desktop, this
>   mechanism will trigger tty1 to set the led state. If other tty
>   lights up the led of the keyboard before switching to the desktop,
>   the led will be forcibly turned off. This situation should
>   be avoided.
> 
> * The current patch is to solve these problems: by judging the
>   kb->kbdmode value of the current tty, it is determined whether
>   the led state is forced to be set when the VT switch is completed.

Thanks for the update, looks pretty much good!

> Signed-off-by: lianzhi chang <changlianzhi@...ontech.com>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
>  v18:
>  (1) Modify the description information.
>  (2) The judgment condition of kb->kbdmode should be "!=",
>  correct it.

>  (3) Modify code indentation.

It's still not updated.

>  drivers/tty/vt/keyboard.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index c7fbbcdcc346..bcf8a858afd2 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -153,6 +153,7 @@ static int shift_state = 0;
>  
>  static unsigned int ledstate = -1U;			/* undefined */
>  static unsigned char ledioctl;
> +static bool vt_switch;
>  
>  /*
>   * Notifier list for console keyboard events
> @@ -412,9 +413,21 @@ static void do_compute_shiftstate(void)
>  /* We still have to export this method to vt.c */
>  void vt_set_leds_compute_shiftstate(void)
>  {
> +	struct kbd_struct *kb;
>  	unsigned long flags;
>  
> -	set_leds();
> +	/*
> +	 * When switching VT, according to the value of kb->kbdmode,
> +	 * judge whether it is necessary to force the keyboard light
> +	 * state to be issued.
> +	 */
> +	kb = kbd_table + fg_console;
> +	if (kb->kbdmode != VC_RAW ||

> +		 kb->kbdmode != VC_MEDIUMRAW ||
> +		 kb->kbdmode != VC_OFF) {

These two are still too right shifted.

> +		vt_switch = true;
> +		set_leds();
> +	}
>  
>  	spin_lock_irqsave(&kbd_event_lock, flags);
>  	do_compute_shiftstate();
> @@ -1255,6 +1268,11 @@ static void kbd_bh(struct tasklet_struct *unused)
>  	leds |= (unsigned int)kbd->lockstate << 8;
>  	spin_unlock_irqrestore(&led_lock, flags);
>  
> +	if (vt_switch) {
> +		ledstate = ~leds;
> +		vt_switch = false;
> +	}
> +
>  	if (leds != ledstate) {
>  		kbd_propagate_led_state(ledstate, leds);
>  		ledstate = leds;
> -- 
> 2.20.1

-- 
With Best Regards,
Andy Shevchenko