Re: [PATCH] tty: n_null: remove legacy BUG_ON() from module initialization

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] tty: n_null: unreachable error path and inaccurate changelog in
  n_null_init()
--

commit 2e48b6bbf8be04b1c87e0588bfba668806cfe4f9
Author: Bramwel Barack <bramwelbarack89@gmail.com>

tty: n_null: remove legacy BUG_ON() from module initialization

This commit replaces the BUG_ON() call in n_null_init() with an error check
and pr_err() log when registering null_ldisc.

> diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
> index 5a429d923eb31..886f6789f872c 100644
> --- a/drivers/tty/n_null.c
> +++ b/drivers/tty/n_null.c
> @@ -32,7 +32,14 @@ static struct tty_ldisc_ops null_ldisc = {
>  
>  static int __init n_null_init(void)
>  {
> -	BUG_ON(tty_register_ldisc(&null_ldisc));
> +	int ret;
> +
> +	ret = tty_register_ldisc(&null_ldisc);
> +	if (ret) {
> +		pr_err("n_null: failed to register line discipline (%d)\n", ret);
> +		return ret;
> +	}
> +
>  	return 0;
>  }

[Severity: Low]
Can tty_register_ldisc(&null_ldisc) ever return a non-zero value here?

The commit message says tty_register_ldisc() can fail if the slot is
already occupied, and that n_null_init() propagates the error to the module
loader.  However, tty_register_ldisc() never checks whether a slot is
occupied; it only returns -EINVAL when new_ldisc->num is outside [N_TTY,
NR_LDISCS) and otherwise overwrites tty_ldiscs[new_ldisc->num]:

drivers/tty/tty_ldisc.c:tty_register_ldisc() {
	if (new_ldisc->num < N_TTY || new_ldisc->num >= NR_LDISCS)
		return -EINVAL;

	raw_spin_lock_irqsave(&tty_ldiscs_lock, flags);
	tty_ldiscs[new_ldisc->num] = new_ldisc;
	raw_spin_unlock_irqrestore(&tty_ldiscs_lock, flags);

	return 0;
}

Since null_ldisc.num is statically initialized to N_NULL (27), the if (ret)
branch and pr_err() are unreachable, and n_null.o is always built-in under
CONFIG_TTY rather than a loadable module.  If BUG_ON() is dropped,
n_null_init() can simply return tty_register_ldisc(&null_ldisc).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108125031.33753-1-bramwelbarack89@gmail.com?part=1