On Tue, Jun 06, 2023 at 04:51:13PM +0300, Dan Carpenter wrote:
> On Tue, Jun 06, 2023 at 04:16:48PM +0300, Andy Shevchenko wrote:
> > On Tue, Jun 06, 2023 at 11:26:25AM +0300, Dan Carpenter wrote:
> > > The put_device() function will call serial_base_ctrl_release() or
> > > serial_base_port_release() so these kfrees() are a double free bug.
...
> > These labels are also called without device being even added.
> > So, this is not good enough as far as I can tell.
>
> put_device() matches with the device_initialize() in
> serial_base_device_init(). If we wanted to undo a device_add() the
> function is device_del().
>
> I originally wrote this patch last week but only resent it now because
> of an issue with my mail set up. I see that serial_base_device_init()
> has actually changed and there is an issue now where the -EPROBE_DEFER
> path leaks.
>
> I think making callers handle -EPROBE_DEFER as a special case would be
> an ongoing source of bugs. So really I'd prefer something like this:
I'm okay with the above, but it seems at the same time we need to limit the
messages:
dev_err_once(port->dev, "uart_add_one_port() called before arch_initcall()?\n");
> regards,
> dan carpenter
>
> diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c
> index 9354af7c11af..15fa0576d362 100644
> --- a/drivers/tty/serial/serial_base_bus.c
> +++ b/drivers/tty/serial/serial_base_bus.c
> @@ -50,17 +50,17 @@ static int serial_base_device_init(struct uart_port *port,
> void (*release)(struct device *dev),
> int id)
> {
> - if (!serial_base_initialized) {
> - dev_err(port->dev, "uart_add_one_port() called before arch_initcall()?\n");
> - return -EPROBE_DEFER;
> - }
> -
> device_initialize(dev);
> dev->type = type;
> dev->parent = parent_dev;
> dev->bus = &serial_base_bus_type;
> dev->release = release;
>
> + if (!serial_base_initialized) {
> + dev_err(port->dev, "uart_add_one_port() called before arch_initcall()?\n");
> + return -EPROBE_DEFER;
> + }
> +
> return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id);
> }
>
>
--
With Best Regards,
Andy Shevchenko
