linux-kernel - Re: [PATCH v9] 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: <YYAXVP4v2bCpGA8s@google.com>
Date:   Mon, 1 Nov 2021 09:35:32 -0700
From:   "dmitry.torokhov" <dmitry.torokhov@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     常廉志 <changlianzhi@...ontech.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        jirislaby <jirislaby@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        282827961 <282827961@...com>
Subject: Re: [PATCH v9] tty: Fix the keyboard led light display problem

On Mon, Nov 01, 2021 at 01:48:25PM +0100, Greg KH wrote:
> On Mon, Nov 01, 2021 at 08:35:47PM +0800, 常廉志 wrote:
> > > Switching from the desktop environment to the tty environment,
> > > the state of the keyboard led lights and the state of the keyboard
> > > lock are inconsistent. This is because the attribute kb->kbdmode
> > > of the tty bound in the desktop environment (Xorg) is set to
> > > VC_OFF, which causes the ledstate and kb->ledflagstate
> > > values of the bound tty to always be 0, which causes the switch
> > > from the desktop When to the tty environment, the LED light
> > > status is inconsistent with the keyboard lock status.
> > > 
> > > Signed-off-by: lianzhi chang <changlianzhi@...ontech.com>
> > > ---
> > >  v7-->v8:
> > >  Optimize the implementation of kbd_update_ledstate function
> >  > 
> > >  Why not adopt the opinions of Greg KH and Andy Shevchenko:
> > >  (1) In the structure struct input_dev, the definition of led is
> > >  like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
> > >  define it like this: unsigned long newstate = *dev->led; I
> > >  always feel that there is still big end and Little endian problem.
> > >  (2) The test_bit function is used to avoid the problem of large
> > >  and small ends, and the current algorithm (v8) also exists
> > >  elsewhere in the kernel: the atkbd_set_leds function (drivers/
> > >  input/keyboard/atkbd.c).
> > >  (3) In the current keyboard.c code, the code is already very good,
> > >  and it is already relatively independent. If you modify the type
> > >  of ledstate to u64 or modify the macro definitions such as
> > >  VC_NUMLOCK, it feels that it is not very meaningful, and this It
> > >  will also cause other related modifications. Of course, this is
> > >  only my current opinion. If everyone still feels that it is
> > >  necessary to modify, I will do it this way. Of course, this
> > >  process may be a bit longer, and I think it is necessary to
> > >  conduct more tests.
> > >  
> > >  v9: Change description information: xorg-->Xorg
> > > ……
> > 
> > Hi, friends, I would like to ask whether this version of patch is possible, if not,
> > I will try my best to find a way to complete the next version!
> 
> It's the merge window right now, we can't do anything with this until
> after 5.16-rc1 is out.  So give us until then at the least please, then
> I will review it again.

As I mentioned, the state of physical LED on a keyboard does not
necessarily reflect state of logical LED state in tty/vt. One can assign
LED on their keyboard to be an indicator of being connected to mains by
assigning "AC-trigger" to it. So the way this patch tries to fix the
issue (by reading internal state of an individual input device) is
wrong.

We keep separate led and lock states for each VT instance, and they
should be applied when switching to/from a VT. Are you saying that in
certain scenarios this switch is not happening? Can see if that can be
fixed?

Thanks.

-- 
Dmitry