linux-kernel - Re: complete_all and "forever" completions

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: <20161027095127.gl464wgwonssk7ry@nobby.bmw-carit.de>
Date:   Thu, 27 Oct 2016 11:51:28 +0200
From:   Daniel Wagner <wagi@...om.org>
To:     Nicholas Mc Guire <der.herr@...r.at>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        LKML <linux-kernel@...r.kernel.org>, Tejun Heo <tj@...nel.org>,
        computersforpeace@...il.com, Ingo Molnar <mingo@...nel.org>
Subject: Re: complete_all and "forever" completions

On Wed, Oct 26, 2016 at 09:15:19AM +0000, Nicholas Mc Guire wrote:
> On Wed, Oct 26, 2016 at 10:45:35AM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 25, 2016 at 03:30:54PM -0700, Dmitry Torokhov wrote:
> > > Or do we need something like this in
> > > do_wait_for_common():
> > > 
> > > 	if (x->done < UINT_MAX/2)
> > > 		x->done--;
> > 
> > Depends a bit, do you really want this? Seems a bit daft to keep asking
> > if its done already, seems like a waste of cycles to me.
> > 
> 
> I would claim that if you have a complete_all() (done=2147483648) and you
> actually did manage to decrement it to 0 over time so a call finally blocks
> (presumably for ever) this would be uncovering a deisgn bug in the use of
> completion as such a setup does not make any sense (Or Im just not creative
> enough to think of such a situation).

I am reviewing all the complete_all() users in order to figure out if
we could weaken the garantees which complete_all() gives you: can be
used in hard irq context and irq disabled context. But that is a
different story.

So while doing the review I found things like

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:

vchiq_arm_init_state() {
	[...]

		init_completion(&arm_state->vc_resume_complete);
		/* Initialise to 'done' state.  We only want to block on resume
		 * completion while videocore is suspended. */
		set_resume_state(arm_state, VC_RESUME_RESUMED);

		init_completion(&arm_state->resume_blocker);
		/* Initialise to 'done' state.  We only want to block on this
		 * completion while resume is blocked */
		complete_all(&arm_state->resume_blocker);

		init_completion(&arm_state->blocked_blocker);
		/* Initialise to 'done' state.  We only want to block on this
		 * completion while things are waiting on the resume blocker */
		complete_all(&arm_state->blocked_blocker);

	[...]
}

If I read this corredtly, there are some 'interesting' uses of
completion where you might run into limits.

cheers,
daniel