Re: [PATCH 2/2] braille: nbcon: Use nbcon atomic console callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] braille: operator precedence bug disables braille atomic write
- [High] braille: infinite spinlock in braille_write() during same-CPU unsafe takeover
--

commit a7f38c0383481bd6ad9c244405d8d201d604569b
Author: Petr Mladek <pmladek@suse.com>

braille: nbcon: Use nbcon atomic console callbacks

This commit adapts the braille console driver to support the new NBCON API.
It introduces logic to acquire the nbcon console ownership before printing
and uses the atomic write callback to output the buffer. The serial8250
driver is also adjusted to skip prepending newlines for braille consoles.

> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
> --- a/drivers/accessibility/braille/braille_console.c
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -62,14 +62,32 @@ static void braille_write(u16 *buf)

[ ... ]

> +	if (braille_co->flags & CON_NBCON) {
> +		/*
> +		 * Braille console might be called from unknown context via
> +		 * vt_console_print() from console_unlock() from printk().
> +		 * Use the atomic callback and synchronize it just using
> +		 * the console context. Disable interrupts to prevent a nested
> +		 * call into the driver code which might cause a deadlock when
> +		 * trying to acquire the console ownership, see
> +		 * __nbcon_atomic_flush_pending_con().
> +		 */
> +		local_irq_save(flags);
> +		while (!nbcon_braille_try_acquire(braille_co, &wctxt))
> +			cpu_relax();

[Severity: High]
Could this loop become an infinite spinlock if triggered from a nested
context on the same CPU?

If an interrupt preempts the underlying nbcon console driver (such as 8250)
while it is in an unsafe region, and that interrupt triggers a printk(),
the legacy VT console might be flushed via console_unlock(), eventually
calling braille_write().

Since local interrupts are disabled here, the preempted nbcon context on
this same CPU can never resume to release the console. Because
nbcon_context_try_acquire() rejects handovers to the same CPU when the
console is in an unsafe state, won't this loop spin indefinitely?

[ ... ]

> -	braille_co->write(braille_co, data, c - data);
> +	if (braille_co->flags & CON_NBCON) {
> +		if (braille_co->write_atomic &&
> +		    !braille_co->flags & CON_NBCON_ATOMIC_UNSAFE) {

[Severity: High]
Does this operator precedence evaluate as intended?

Because the logical not (!) operator has higher precedence than the bitwise
and (&) operator, this expression parses as:

(!braille_co->flags) & CON_NBCON_ATOMIC_UNSAFE

Since braille_co->flags is known to be non-zero (as CON_NBCON was just
checked above), evaluating the logical not against the flags yields 0. This
makes the condition permanently false, which bypasses the braille atomic
write callback unconditionally.

Should this be grouped with parentheses to evaluate the bitwise and first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922072558.98854-1-pmladek@suse.com?part=2