Re: [RESEND PATCH 2/2] device property: fix for a case of use-after-free

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

Hi Rafael,

> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index a163f2c..a9df21a9 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -820,11 +820,13 @@ void device_remove_property_set(struct device *dev)
> >  	 * the pset. If there is no real firmware node (ACPI/DT) primary
> >  	 * will hold the pset.
> >  	 */
> > -	if (!is_pset_node(fwnode))
> > +	if (is_pset_node(fwnode))
> > +		dev->fwnode = NULL;
> 
> I don't really like the way you clear dev->fwnode directly here.
> set_primary_fwnode(dev, NULL) would be more appropriate IMO.
> 
> Also set_secondary_fwnode(dev, NULL) need not be done in that case, because it
> doesn't change anything.
> 
> Moreover, if the primary node is not pset, the secondary one should only be
> cleared if it is pset.
> 
> So that would mean
> 
> 	if (is_pset_node(fwnode)) {
> 		set_primary_fwnode(dev, NULL);
> 		pset_free_set(to_pset_node(fwnode));
> 	} else {
> 		fwnode = fwnode->secondary;
> 		if (!IS_ERR(fwnode) && is_pset_node(fwnode)) {
> 			set_secondary_fwnode(dev, NULL);
> 			pset_free_set(to_pset_node(fwnode));
> 		}
> 	}

I have been testing that today, but I'll continue tomorrow. No
problems so far.


Thanks,

-- 
heikki