linux-kernel - Re: [PATCH 4/6] staging: vme: add functions for bridge module refcounting

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: <20110810191424.GA26147@flamenco.cs.columbia.edu>
Date:	Wed, 10 Aug 2011 15:14:24 -0400
From:	"Emilio G. Cota" <cota@...ap.org>
To:	Martyn Welch <martyn.welch@...com>
Cc:	Manohar Vanga <manohar.vanga@...n.ch>, gregkh@...e.de,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] staging: vme: add functions for bridge module
 refcounting

On Wed, Aug 10, 2011 at 11:09:56 +0100, Martyn Welch wrote:
> On 10/08/11 10:33, Manohar Vanga wrote:
> >  static struct vme_bridge *find_bridge(struct vme_resource *resource)
> > @@ -1525,9 +1573,13 @@ static int vme_bus_probe(struct device *dev)
> >  	driver = dev_to_vme_driver(dev);
> >  	bridge = dev_to_bridge(dev);
> >  
> > +	vme_bridge_get(bridge->num);

Here the return pointer is not being checked, although things
would need to be very wrong for this to fail here.

> >  	if (driver->probe != NULL)
> >  		retval = driver->probe(dev, bridge->num, vme_calc_slot(dev));
> >  
> > +	if (retval)
> > +		vme_bridge_put(bridge);
> > +
> 
> Given that we are recounting automatically in probe and remove, under what
> circumstances would we need to call them separately?

I agree, it's hard to imagine a scenario where "doubly locking"
the module would make any sense at all.

It would also be nice to:

- Increment the refcount of the bridge device with get_device(),
  this way we protect against the device removal as well.
  Bonus points if we increment the bridge module's refcount
  once per device driver using it and the bridge device's
  refcount once per device sitting on it.

- re-think what the return value of vme_bridge_get should be;
  guess my original idea was to return a reference to a bridge,
  which would be safely used by device drivers from there
  on. Since apparently we're not going this way, then perhaps
  an int (returning 0 on success and -ENXIO on failure) would
  be clearer.

		Emilio
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/