Re: [PATCH] serial: core: don't kfree device managed data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux