Re: [PATCH v4 1/1] braille: nbcon: Allow to use a serial console with NBCON API as Braille console

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] console: deadlock in braille_write() from atomic/IRQ context
- [High] console: infinite loop in braille_write() when CONFIG_PRINTK is disabled
--

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

braille: nbcon: Allow to use a serial console with NBCON API as Braille console

This patch integrates the Braille console with the NBCON API by allowing
braille_write() to attempt acquiring NBCON ownership before writing. It
simulates the original locking using nbcon_braille_try_acquire() and routes
output through write_atomic() or write_thread() callbacks.

> 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,50 @@ static void braille_write(u16 *buf)
[ ... ]
> +	if (braille_co->flags & CON_NBCON) {
> +		if (panic_on_this_cpu()) {
> +			/*
> +			 * This should be good enough in practice. Most/all
> +			 * serial console drivers have the atomic callback.
> +			 */
> +			if (!braille_co->write_atomic)
> +				return;
> +
> +			local_irq_save(flags);
> +			/* NBCON API strictly requires the ownership. */
> +			if (!nbcon_braille_try_acquire(braille_co, &wctxt)) {
> +				local_irq_restore(flags);
> +				return;
> +			}
> +		} else {
> +			braille_co->device_lock(braille_co, &flags);
> +			while (!nbcon_braille_try_acquire(braille_co, &wctxt))
> +				cpu_relax();
> +		}
> +	}

[Severity: High]
Could this lead to a deadlock? braille_write() can be invoked by VT
notifiers from atomic or IRQ contexts (e.g., from keyboard interrupts or
console_flush_all()). By calling device_lock() and spinning on NBCON
ownership with a cpu_relax() loop, if the interrupted context already holds
the lock (like an active nbcon flush), nbcon_braille_try_acquire() will
permanently return false, causing an infinite busy-loop.

[ ... ]
> @@ -102,7 +138,24 @@ static void braille_write(u16 *buf)
[ ... ]
> -	braille_co->write(braille_co, data, c - data);
> +	if (braille_co->flags & CON_NBCON) {
> +		nbcon_write_context_set_buf(&wctxt, (char *)data, c - data);
> +		if (panic_on_this_cpu())
> +			braille_co->write_atomic(braille_co, &wctxt);
> +		else
> +			braille_co->write_thread(braille_co, &wctxt);

[Severity: High]
Does this violate the write_thread API constraints? The
include/linux/console.h documentation specifies that write_thread must
strictly execute in a task context. Since braille_write() can be called
from an atomic or IRQ context, invoking the kthread-specific callback here
could be unsafe.

> +	} else {
> +		braille_co->write(braille_co, data, c - data);
> +	}
[ ... ]
> diff --git a/include/linux/console.h b/include/linux/console.h
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
[ ... ]
> @@ -678,8 +687,14 @@ static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
>  static inline bool nbcon_kdb_try_acquire(struct console *con,
>  					 struct nbcon_write_context *wctxt) { return false; }
>  static inline void nbcon_kdb_release(struct nbcon_write_context *wctxt) { }
> +static inline
> +bool nbcon_write_context_is_braille(struct nbcon_write_context *wctxt) { return false; }
>  static inline bool console_is_usable(struct console *con, short flags,
>  				     bool use_atomic) { return false; }
> +static inline bool nbcon_braille_try_acquire(struct console *con,
> +					     struct nbcon_write_context *wctxt) { return false; }

[Severity: High]
Could this cause a hard lockup if CONFIG_A11Y_BRAILLE_CONSOLE is enabled
but CONFIG_PRINTK is disabled?

Since this fallback stub unconditionally returns false, the
while (!nbcon_braille_try_acquire(...)) cpu_relax(); loop in braille_write()
will never terminate when triggered by terminal output.

> +static inline void nbcon_braille_release(struct nbcon_write_context *wctxt) { }
> +static inline bool nbcon_is_braille(struct console *con) { return false; }
>  #endif

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