All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Cen Zhang <zzzccc427@gmail.com>
Cc: jirislaby@kernel.org, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org, baijiaju1990@gmail.com
Subject: Re: [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet
Date: Sun, 10 May 2026 18:55:14 +0200	[thread overview]
Message-ID: <2026051039-snitch-attitude-a67d@gregkh> (raw)
In-Reply-To: <CAFRLqsUeat=rUj7aCSAku=A9NBNj3Wxd+xi8xzzBtRWqCxp18Q@mail.gmail.com>

On Mon, May 11, 2026 at 12:43:26AM +0800, Cen Zhang wrote:
> Hi Greg,
> 
> > On Sun, May 10, 2026 at 06:22:xxPM +0200, Greg KH wrote:
> 
> > Why is that an issue?
> 
> Thanks, that's a fair point. The issue I was trying to describe is not
> that KDGETLED can race with a normal LED state transition and return
> either the old or the new value. That part is just the usual snapshot
> semantics, and the value can of course change immediately after the ioctl
> returns.
> 
> The specific case I was concerned about is the VT-switch force-update
> path. vt_set_leds_compute_shiftstate() sets vt_switch and schedules
> keyboard_tasklet, and then kbd_bh() does:
> 
>     if (vt_switch) {
>             ledstate = ~leds;
>             vt_switch = false;
>     }
> 
>     if (leds != ledstate) {
>             kbd_propagate_led_state(ledstate, leds);
>             ledstate = leds;
>     }
> 
> That first assignment is only an internal old-state sentinel to force LED
> propagation when switching VTs. It is not a committed LED state intended
> to be returned to userspace. Since KDGETLED currently returns
> ledstate & 0xff without quiescing the tasklet, it can expose that
> temporary complemented value. For example, if the real LED state is
> LED_NUM only, a read in that window can return the complemented low byte
> instead of 0x02.
> 
> > stable for "what" exactly? This is a snapshot in time, be it before or
> > after it changes is not always going to really matter here, as it can
> > change right after you "enable" the tasklet, right?
> 
> Yes, I agree. "stable snapshot" was too broad a description.
> 
> By "stable" I only meant "not while kbd_bh() is in the middle of using
> that internal force-update value". tasklet_disable() would make KDGETLED
> observe either the old value before the tasklet runs, or the committed
> value after it finishes. It does not make the value stable after
> tasklet_enable(), and the changelog should not imply that.
> 
> This is a small userspace-visible consistency issue, not a memory-safety
> problem. If this behavior is considered acceptable for KDGETLED, I can
> drop the patch; otherwise I can send a v2 with a narrower changelog.

In the past decades of use, has anyone every noticed or complained about
this consitency issue?  If not, I would recommend not worrying about it :)

thanks,

greg k-h

  reply	other threads:[~2026-05-10 16:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10  3:13 [PATCH] vt: keyboard: serialize KDGETLED with keyboard_tasklet Cen Zhang
2026-05-10 16:21 ` Greg KH
2026-05-10 16:43   ` Cen Zhang
2026-05-10 16:55     ` Greg KH [this message]
2026-05-10 17:01       ` Cen Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026051039-snitch-attitude-a67d@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=baijiaju1990@gmail.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=zzzccc427@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.