Re: [PATCH] tty: Serialize saved termios access with device registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tty: mixing goto-based error handling with scoped cleanup helpers
--

commit 6bd1bebdb72fb484cd4c55333cfb5b66866fd06e
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

tty: Serialize saved termios access with device registration

tty_register_device_attr() frees saved termios data when reusing a minor
number without serializing with tty_init_termios() or tty_save_termios().
The tty_mutex held by the open and close paths does not protect against
this registration-side free.

> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 48569035da56e..26bdc4560a4c8 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
[ ... ]
> @@ -3242,10 +3247,12 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
>  		 * Free any saved termios data so that the termios state is
>  		 * reset when reusing a minor number.
>  		 */
> -		tp = driver->termios[index];
> -		if (tp) {
> -			driver->termios[index] = NULL;
> -			kfree(tp);
> +		scoped_guard(mutex, &tty_termios_mutex) {
> +			tp = driver->termios[index];
> +			if (tp) {
> +				driver->termios[index] = NULL;
> +				kfree(tp);
> +			}
>  		}
>  
>  		retval = tty_cdev_add(driver, devt, index, 1);

[Severity: Low]
Does this violate the guidelines against mixing goto-based error handling
with scope-based cleanup helpers?

tty_register_device_attr() already uses explicit goto labels for error
cleanup:

err_del:
	device_del(dev);
err_put:
	put_device(dev);

Mixing these paradigms can create confusing ownership semantics and
increases the risk of resource leaks or double-free bugs during future
refactorings.

Could the function be completely converted to scope-based cleanup first,
or could this patch use traditional locking (mutex_lock/mutex_unlock)
instead?

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