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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210113155504.GS4077@smile.fi.intel.com>
Date:   Wed, 13 Jan 2021 17:55:04 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Daniel Scally <djrscally@...il.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.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 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)?

Looking more into this code I think we need to call

	set_secondary_fwnode(dev, NULL);

among these lines.

The real problem is that set_primary_fwnode() and set_secondary_fwnode() have
no reference counting. If we have a chain ->primary->secondary->-ENODEV is
being used somewhere we can't tell from here.

So, in practice it means that we lack of some kind of primary node to increment
reference count of the secondary node when the latter is chained to the given
primary. But it makes things too complicated. Any other options for shared
primary-secondary chain? Standalone primary along with standalone (exclusive)
secondary doesn't need this AFAICS. Perhaps a flag to primary like shared /
exclusive that will prevent breaking the chain in set_secondary_fwnode()?

-- 
With Best Regards,
Andy Shevchenko