Re: [PATCH 4/7] serial: core: Power the line transceiver with the port

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 2 related pre-existing issues it does not
introduce:
- [Critical] serial: core: use-after-free in uart_line_info() [1]
- [Critical] serial: core: dangling pointer in
  serial_core_add_one_port() [2]
--

commit 54c2531a6ac957209b731d5dba93f7f0ba6a052f
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: core: Power the line transceiver with the port

This commit requests the optional transceiver-enable GPIO and startup delay
for a serial port and toggles the transceiver power alongside the port in
uart_change_pm().

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 6c0a6d9dcb851..d36bffaa100b8 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c

[ ... ]

> @@ -3282,6 +3310,12 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  
>  		if (tty_port_initialized(port))
>  			uart_shutdown(NULL, state);
> +
> +		/* Only a console port is still powered here */
> +		if (uport->transceiver_enable_gpio &&
> +		    state->pm_state == UART_PM_STATE_ON)
> +			uart_change_pm(state, UART_PM_STATE_OFF);
> +		uport->transceiver_enable_gpio = NULL;
>  	}

[Severity: Critical]
This problem wasn't introduced by this patch, but can uart_line_info() race
with serial_core_remove_one_port() and cause a use-after-free [1]?

serial_core_remove_one_port() drops port->mutex at the end of this block and
calls uport->ops->release_port(uport) before setting uport->type to
PORT_UNKNOWN:

CPU0                              CPU1
serial_core_remove_one_port()
  // drops port->mutex
  uport->ops->release_port(uport)
                                  uart_line_info()
                                    guard(mutex)(&port->mutex)
                                    if (uport->type == PORT_UNKNOWN)
                                      return;
                                    uport->ops->get_mctrl(uport)  <-- use-after-free
  uport->type = PORT_UNKNOWN

A concurrent read of /proc/tty/driver/serial sees uport->type != PORT_UNKNOWN
and invokes get_mctrl() on unmapped IO resources.

[ ... ]

> @@ -3409,6 +3453,14 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  err_unregister_ctrl_dev:
>  	serial_base_ctrl_device_remove(new_ctrl_dev);
>  
> +err_put_transceiver:
> +	/* The port may be registered again: forget the GPIO */
> +	if (core_transceiver && port->transceiver_enable_gpio) {
> +		devm_gpiod_put(port->dev, port->transceiver_enable_gpio);
> +		port->transceiver_enable_gpio = NULL;
> +		port->transceiver_startup_delay_us = 0;
> +	}
> +
>  	return ret;
>  }

[Severity: Critical]
This problem wasn't introduced by this patch, but does
serial_core_add_one_port() leave a dangling pointer in uport->name when
allocating uport->tty_groups fails [2]?

drivers/tty/serial/serial_core.c:serial_core_add_one_port() {
	...
	uport->tty_groups = kzalloc_objs(*uport->tty_groups, 3);
	if (!uport->tty_groups) {
		kfree(uport->name);
		return -ENOMEM;
	}
	...
}

Because the caller owns uport across registration failure, leaving the freed
pointer in uport->name can lead to a use-after-free or double free during
driver cleanup or a subsequent registration retry.

[1] https://sashiko.dev/#/bug/linux-dccfb530-78ab-4c75-b86c-353ac639e159
[2] https://sashiko.dev/#/bug/linux-4946dfed-5de8-46a5-b799-8b71897faf37

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-serial-transceiver-supply-v1-0-40bad0f912f2@vaisala.com?part=4