[RFC PATCH 1/3] drivers: of: fix resources freeing in of_pci_get_host_bridge_resources()

[Liviu Dudau]

On Mon, Jan 26, 2015 at 11:21:11AM +0000, Lorenzo Pieralisi wrote:
> On Tue, Jan 20, 2015 at 11:20:32AM +0000, Liviu Dudau wrote:
> > On Tue, Jan 20, 2015 at 10:49:22AM +0000, Lorenzo Pieralisi wrote:
> > > On Mon, Jan 19, 2015 at 06:32:29PM +0000, Liviu Dudau wrote:
> > > 
> > > [...]
> > > 
> > > > > @@ -146,6 +146,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> > > > >  	struct of_pci_range_parser parser;
> > > > >  	char range_type[4];
> > > > >  	int err;
> > > > > +	struct pci_host_bridge_window *window;
> > > > >  
> > > > >  	if (io_base)
> > > > >  		*io_base = (resource_size_t)OF_BAD_ADDR;
> > > > > @@ -225,7 +226,10 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> > > > >  conversion_failed:
> > > > >  	kfree(res);
> > > > >  parse_failed:
> > > > > +	list_for_each_entry(window, resources, list)
> > > > > +		kfree(window->res);
> > > > >  	pci_free_resource_list(resources);
> > > > > +	kfree(bus_range);
> > > > >  	return err;
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> > > > 
> > > > Hi Lorenzo et all,
> > > > 
> > > > Here is my personal view and I am happy to hear from others on the desired
> > > > behaviour:
> > > > 
> > > > When I wrote this function what I had in mind was that it will parse as
> > > > much as possible from the device tree and return a list of resources that
> > > > could be successfully converted. If the entire list of ranges could not
> > > > be converted then an error code will be returned, but the caller still
> > > > had the list as constructed up to the error. It was the job of the caller
> > > > to free the list in either cases, as stated in the comment.
> > > 
> > > That's what I am questioning. If the function takes an error path, the
> > > windows list is freed, so the resource pointers are gone. There is no
> > > way the caller can grab those resource pointers and free them.
> > 
> > I stand corrected. Your patch is needed.
> 
> Mind acking it ? I will ask Bjorn to take it then.

Sure. 

Acked-by: Liviu Dudau <Liviu.Dudau at arm.com>

> 
> Thanks,
> Lorenzo
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯