linux-kernel - Re: [PATCH v11 3/4] add FPGA manager core

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]

Message-ID: <20150923230315.GF26335@jcartwri.amer.corp.natinst.com>
Date:	Wed, 23 Sep 2015 18:03:15 -0500
From:	Josh Cartwright <joshc@...com>
To:	atull <atull@...nsource.altera.com>
Cc:	gregkh@...uxfoundation.org, jgunthorpe@...idianresearch.com,
	hpa@...or.com, monstr@...str.eu, michal.simek@...inx.com,
	rdunlap@...radead.org, Moritz Fischer <moritz.fischer@...us.com>,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	pantelis.antoniou@...sulko.com, robh+dt@...nel.org,
	grant.likely@...aro.org, iws@...o.caltech.edu,
	linux-doc@...r.kernel.org, pavel@...x.de, broonie@...nel.org,
	philip@...ister.org, rubini@...dd.com, s.trumtrar@...gutronix.de,
	jason@...edaemon.net, kyle.teske@...com, nico@...aro.org,
	balbi@...com, m.chehab@...sung.com, davidb@...eaurora.org,
	rob@...dley.net, davem@...emloft.net, cesarb@...arb.net,
	sameo@...ux.intel.com, akpm@...ux-foundation.org,
	linus.walleij@...aro.org, pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	devel@...verdev.osuosl.org, Petr Cvek <petr.cvek@....cz>,
	delicious.quinoa@...il.com, dinguyen@...nsource.altera.com
Subject: Re: [PATCH v11 3/4] add FPGA manager core

On Wed, Sep 23, 2015 at 12:10:13PM -0500, atull wrote:
> On Tue, 22 Sep 2015, Josh Cartwright wrote:
[..]
> > > +struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
> > > +{
> > > +	struct fpga_manager *mgr;
> > > +	struct device *dev;
> > > +
> > > +	if (!node)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	dev = class_find_device(fpga_mgr_class, NULL, node,
> > > +				fpga_mgr_of_node_match);
> > > +	if (!dev)
> > > +		return ERR_PTR(-ENODEV);
> > > +
> > > +	mgr = to_fpga_manager(dev);
> > > +	put_device(dev);
> > 
> > Who's ensuring the FPGA manager device's lifetime between _get and _put?
> 
> Well... get_device and put_device are not sufficient?

Sorry if I was too opaque.

You've just put_device()'d the only reference to the device you had
here, so nothing is preventing from the device from being ripped away
while it's being used.

You should remove the put_device() call here, and add a corresponding
put_device() in fpga_mgr_put().

The module refcounting is also a bit strange, as you are
acquiring/releasing a reference to THIS_MODULE, but you also should care
about the lower-level driver's module refcount.

[..]
> > > +	dt_label = of_get_property(mgr->dev.of_node, "label", NULL);
> > > +	if (dt_label)
> > > +		ret = dev_set_name(&mgr->dev, "%s", dt_label);
> > > +	else
> > > +		ret = dev_set_name(&mgr->dev, "fpga%d", id);
> > 
> > I'm wondering if an alias {} node is better for this.
> 
> I could look into that.  Is there an example of that you particularly
> like for this?

Look at i2c's usage of the of_alias_*() functions.

  Josh

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)