Hi,
On Mon, Aug 5, 2024 at 7:28 AM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
> > Cc: stable@xxxxxxxxxxxxxxx
> > Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> > 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.
