Hello Yegor,
On Tue, Apr 05, 2016 at 12:32:53PM +0200, Yegor Yefremov wrote:
> I've got a kernel crash from kernel robot. If we use UART before
> general initialization (earlyprintk), then any call to mctrl API would
> result in NULL pointer dereference. One solution would be to check, if
> gpios IS_ERR_OR_NULL(). See below:
>
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -54,6 +54,9 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios,
> unsigned int mctrl)
> int value_array[UART_GPIO_MAX];
> unsigned int count = 0;
>
> + if (IS_ERR_OR_NULL(gpios))
> + return;
> +
> for (i = 0; i < UART_GPIO_MAX; i++)
> if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
> desc_array[count] = gpios->gpio[i];
IS_ERR_OR_NULL(gpios) should never be true. gpios should be the value
that was returned by mctrl_gpio_init, this never returns NULL and if it
returns an error you're supposed to not register the port. And for early
printk there is AFAIK no mctrl involved.
> > static inline int serial8250_in_MCR(struct uart_8250_port *up)
> > {
> > - return serial_in(up, UART_MCR);
> > + int mctrl, mctrl_gpio = 0;
> > +
> > + mctrl = serial_in(up, UART_MCR);
> > +
> > + mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
I forgot where mctrl_gpio_get_outputs came from, is it better
than/different from mctrl_gpio_get?
> > +
> > + if (mctrl_gpio & TIOCM_RTS)
> > + mctrl |= UART_MCR_RTS;
> > + else
> > + mctrl &= ~UART_MCR_RTS;
> > +
> > + if (mctrl_gpio & TIOCM_DTR)
> > + mctrl |= UART_MCR_DTR;
> > + else
> > + mctrl &= ~UART_MCR_DTR;
> > +
> > + if (mctrl_gpio & TIOCM_OUT1)
> > + mctrl |= UART_MCR_OUT1;
> > + else
> > + mctrl &= ~UART_MCR_OUT1;
> > +
> > + if (mctrl_gpio & TIOCM_OUT2)
> > + mctrl |= UART_MCR_OUT2;
> > + else
> > + mctrl &= ~UART_MCR_OUT2;
>
> Should we perhaps check, if particular signal is really implemented as GPIO?
The intended usage is:
mctrl = read_mctrl_signals_from_hardware(port);
return mctrl_gpio_get(gpios, &mctrl);
mctrl_gpio_get then overwrites all values it is responsible for.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
