linux-kernel - Re: [RFC 01/15] drivers/base: add track framework

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:	Mon, 15 Dec 2014 12:55:57 +0000
From:	Mark Brown <broonie@...nel.org>
To:	AH <najdrozszy@...il.com>
Cc:	Andrzej Hajda <a.hajda@...sung.com>,
	open list <linux-kernel@...r.kernel.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Mike Turquette <mturquette@...aro.org>,
	Russell King <linux@....linux.org.uk>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Inki Dae <inki.dae@...sung.com>,
	Kishon Vijay Abraham I <kishon@...com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	ARM/CLKDEV SUPPORT <linux-arm-kernel@...ts.infradead.org>,
	GPIO SUBSYSTEM <linux-gpio@...r.kernel.org>,
	DRM PANEL DRIVERS <dri-devel@...ts.freedesktop.org>,
	"ARM/S5P EXYNOS AR..." <linux-samsung-soc@...r.kernel.org>,
	"OPEN FIRMWARE AND..." <devicetree@...r.kernel.org>
Subject: Re: [RFC 01/15] drivers/base: add track framework

On Sat, Dec 13, 2014 at 12:12:19AM +0100, AH wrote:
> Mark Brown wrote on 12.12.2014 17:36:
> >On Wed, Dec 10, 2014 at 04:48:19PM +0100, Andrzej Hajda wrote:

> >>+		kfree(ptask);
> >>+
> >>+		if (empty)
> >>+			break;
> >>+
> >>+		track_process_task(track, task);

> >...we then go and do some other stuff, including processing that task,
> >without the lock or or any other means I can see of excluding other
> >users before going round and removing the task.  This seems to leave us
> >vulnerable to double execution.

> No, if you look at track_add_task function you will see that the queue is
> processed only if it is initially empty, otherwise the task is only added to
> the queue, so it will be processed after processing earlier tasks.
> So the rule is that if someone add task to the queue it checks if the queue
> is empty, in such case it process all tasks from the queue until
> the queue becomes empty, even the tasks added by other processed.
> This way all tasks are serialized.

This is all pretty fiddly and seems fragile - if nothing else the code
seems undercommented since the above is only going to be apparent with
following through multiple functions and we're relying on both owner and
list emptiness with more than one place where a task can get processed.

> >I'm also unclear what is supposed to happen if adding a notification
> >races with removing the thing being watched.

> The sequence should be always as follows:
> 1. create thing, then call track_up(thing).
> ...
> 2. call track_down(thing) then remove thing.

> If we put 1 into probe and 2 into remove callback of the driver it will be
> safe - we are synchronised by device_lock. But if, for some reason, we want
> to create object after probe we should do own synchronization or just put
> device_lock around 1. The same applies if we want to remove
> object earlier. This is the comment above about. I will expand it to more
> verbose explanation.

You can't rely on the device lock here since this isn't tied to kobjects
or anything at all - it's a freestanding interface someone could pick up
and use in another context.  Besides, that isn't really my concern - my
concern is what happens if something asks to wait for 

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