linux-kernel - Re: [PATCH v2 1/5] driver core: add a faux bus for use when a simple device/bus is needed

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] [day] [month] [year] [list]

Message-ID: <2025020638-refresh-pager-cdc0@gregkh>
Date: Thu, 6 Feb 2025 17:19:48 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <quic_zijuhu@...cinc.com>
Cc: linux-kernel@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, Lyude Paul <lyude@...hat.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Liam Girdwood <lgirdwood@...il.com>, Lukas Wunner <lukas@...ner.de>,
	Mark Brown <broonie@...nel.org>,
	Maíra Canal <mairacanal@...eup.net>,
	Robin Murphy <robin.murphy@....com>,
	Simona Vetter <simona.vetter@...ll.ch>, linux-usb@...r.kernel.org,
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 1/5] driver core: add a faux bus for use when a simple
 device/bus is needed

On Thu, Feb 06, 2025 at 11:34:27PM +0800, Zijun Hu wrote:
> On 2/4/2025 7:09 PM, Greg Kroah-Hartman wrote:
> > +#define MAX_NAME_SIZE	256	/* Max size of a faux_device name */
> > +
> > +/*
> > + * Internal wrapper structure so we can hold the memory
> > + * for the driver and the name string of the faux device.
> > + */
> > +struct faux_object {
> > +	struct faux_device faux_dev;
> > +	const struct faux_driver_ops *faux_ops;
> > +	char name[];
> 
> Remove name since it is not used actually ?

Hm, we do copy it:
	/* Save off the name of the object into local memory */
	memcpy(faux_obj->name, name, name_size);

Ah, but then we do a dev_set_name() so we don't care anymore!  When the
code was a two-step process we did care.  Nice catch, let me go change
that and test it to be sure.

> > +};+ */
> > +void faux_device_destroy(struct faux_device *faux_dev)
> > +{
> > +	struct device *dev = &faux_dev->dev;
> > +
> > +	if (IS_ERR_OR_NULL(faux_dev))
> > +		return;
> > +
> 
> struct device *dev;
> 
> //faux_device_create() does not return ERR_PTR().
> if (!faux_dev)
> 	return;
> 
> // avoid NULL pointer dereference in case of above error
> dev = &faux_dev->dev;

Nope, that wouldn't have been a dereference error, you can set a pointer
to point to NULL just fine as long as you don't try to dereference it to
something else.  Isn't C fun!  :)

> > +	device_del(dev);
> > +
> > +	/* The final put_device() will clean up the driver we created for this device. */
> > +	put_device(dev);
> 
> use device_unregister() instead of above 2 statements?

Could be, both are the same.

> > +}
> > +EXPORT_SYMBOL_GPL(faux_device_destroy);
> > +
> > +int __init faux_bus_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = device_register(&faux_bus_root);
> > +	if (ret) {
> > +		put_device(&faux_bus_root);
> 
> put_device() for static device may trigger below warning:
> 
> drivers/base/core.c:device_release():
> WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is
> broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
> 			dev_name(dev));

Yes, but that will never trigger when you run the code as the final put
device never happens.  So you will not ever see that.

And yes, I HATE static struct devices in the kernel a lot, but in the
driver core we use them for a few things like this, so either I fix all
of them, or just live with the few that we have.

thanks for the review!

greg k-h