Hi,
On Mon, Aug 5, 2024 at 7:28 AM Ilpo Järvinen ilpo.jarvinen@linux.intel.com wrote:
On Mon, 5 Aug 2024, Jiri Slaby (SUSE) wrote:
Coverity reports (as CID 1536978) that uart_poll_init() passes uninitialized pm_state to uart_change_pm(). It is in case the first 'if' takes the true branch (does "goto out;").
Fix this and simplify the function by simple guard(mutex). The code needs no labels after this at all. And it is pretty clear that the code has not fiddled with pm_state at that point.
Signed-off-by: Jiri Slaby (SUSE) jirislaby@kernel.org Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART) Cc: stable@vger.kernel.org Cc: Douglas Anderson dianders@chromium.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/tty/serial/serial_core.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 3afe77f05abf..d63e9b636e02 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2690,14 +2690,13 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options) int ret = 0;
tport = &state->port;
mutex_lock(&tport->mutex);
guard(mutex)(&tport->mutex); port = uart_port_check(state); if (!port || port->type == PORT_UNKNOWN ||
!(port->ops->poll_get_char && port->ops->poll_put_char)) {ret = -1;goto out;}
!(port->ops->poll_get_char && port->ops->poll_put_char))return -1; pm_state = state->pm_state; uart_change_pm(state, UART_PM_STATE_ON);@@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options) ret = uart_set_options(port, NULL, baud, parity, bits, flow); console_list_unlock(); } -out:
if (ret) uart_change_pm(state, pm_state);
mutex_unlock(&tport->mutex);
return ret;}
This too needs #include.
Why? I see in "mutex.h" (which is already included by serial_core.c):
DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
...so we're using the mutex guard and including the header file that defines the mutex guard. Seems like it's all legit to me.