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:
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);
}
