linux-kernel - Re: [PATCH v2 1/3] software node: Introduce device_add_software

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jan 2021 15:19:48 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Daniel Scally <djrscally@...il.com>,
        Felipe Balbi <balbi@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-acpi@...r.kernel.org
Subject: Re: [PATCH v2 1/3] software node: Introduce
 device_add_software_node()

On Wed, Jan 13, 2021 at 05:30:03PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 13, 2021 at 01:39:18PM +0200, Heikki Krogerus wrote:
> > On Wed, Jan 13, 2021 at 12:40:03AM +0000, Daniel Scally wrote:
> > > On 11/01/2021 14:10, Heikki Krogerus wrote:
> 
> ...
> 
> > > > +/**
> > > > + * device_remove_software_node - Remove device's software node
> > > > + * @dev: The device with the software node.
> > > > + *
> > > > + * This function will unregister the software node of @dev.
> > > > + */
> > > > +void device_remove_software_node(struct device *dev)
> > > > +{
> > > > +	struct swnode *swnode;
> > > > +
> > > > +	swnode = dev_to_swnode(dev);
> > > > +	if (!swnode)
> > > > +		return;
> > > > +
> > > > +	kobject_put(&swnode->kobj);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(device_remove_software_node);
> > > 
> > > I wonder if this also ought to set dev_fwnode(dev)->secondary back to
> > > ERR_PTR(-ENODEV)?
> 
> Actually it's a good question.
> 
> > We can't do that here unfortunately. Other places still have a
> > reference to the swnode at this point and they may still need to
> > access it using the dev_fwnode(dev)->secondary pointer.
> 
> Yeah, but in this case we potentially leave a dangling pointer when last of the
> user gone and kobject_put() will call for release.

The caller has to be responsible of setting the secondary back to
ERR_PTR(-ENODEV). We can not do anything here like I explained. We can
not even do that in software_node_notify() when the association to the
struct device is removed, because the fwnode->secondary is still
accessed after that. The caller needs to remove both the node and the
device, and only after that it is safe to set the secondary back to
ERR_PTR(-ENODEV).

This clearly needs to explained in the comment... I'll fix it.


thanks,

-- 
heikki