linux-kernel - Re: [RFC PATCH v7 1/7] Restartable sequences system call

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: <20160810132920.GN30192@twins.programming.kicks-ass.net>
Date:	Wed, 10 Aug 2016 15:29:20 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Russell King <linux@....linux.org.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	linux-api <linux-api@...r.kernel.org>,
	Paul Turner <pjt@...gle.com>, Andrew Hunter <ahh@...gle.com>,
	Andy Lutomirski <luto@...capital.net>,
	Andi Kleen <andi@...stfloor.org>,
	Dave Watson <davejwatson@...com>, Chris Lameter <cl@...ux.com>,
	Ben Maurer <bmaurer@...com>, rostedt <rostedt@...dmis.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Josh Triplett <josh@...htriplett.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Michael Kerrisk <mtk.manpages@...il.com>,
	Boqun Feng <boqun.feng@...il.com>
Subject: Re: [RFC PATCH v7 1/7] Restartable sequences system call

On Tue, Aug 09, 2016 at 08:06:40PM +0000, Mathieu Desnoyers wrote:
> On Aug 3, 2016, at 9:19 AM, Peter Zijlstra peterz@...radead.org wrote:

> >> +++ b/kernel/sched/core.c
> >> @@ -2664,6 +2664,7 @@ prepare_task_switch(struct rq *rq, struct task_struct
> >> *prev,
> >>  {
> >>  	sched_info_switch(rq, prev, next);
> >>  	perf_event_task_sched_out(prev, next);
> >> +	rseq_sched_out(prev);
> > 
> > One thing I considered is doing something like:
> > 
> > static inline void rseq_sched_out(struct task_struct *t)
> > {
> >	unsigned long ptr;
> >	int err;
> > 
> >	if (!t->rseq)
> >		return;
> > 
> >	err = __get_user(ptr, &t->rseq->rseq_cs);
> >	if (err || ptr)
> >		set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
> > }
> > 
> > That will optimistically try to read the rseq_cs pointer and, on success
> > and empty (the most likely case) avoid setting the TIF flag.
> > 
> > This will require an explicit migration hook to unconditionally set the
> > TIF flag such that we keep the cpu_id field correct of course.
> > 
> > And obviously we can do this later, as an optimization. Its just
> > something I figured might be worth it.
> 
> This won't work. The rseq mechanism proposed here is really the overlap
> of _two_ distinct restart mechanisms: a sequence counter for C code,
> and a ip-fixup-based mechanism for the assembly "finish" instruction
> sequence.
> 
> What you propose here only considers the fixup of the assembly instruction
> sequence, but not the C code that runs before. The C code between
> rseq_start() and rseq_finish() loads the current value of the sequence
> counter in rseq_start(), and then it gets compared with the new current
> value within the rseq_finish restartable sequence of instructions. So the
> sequence counter needs to be updated upon preemption/signal delivery that
> occurs on top of C code, even if not nesting over a sequence of
> restartable assembly instructions.

True; we could of course have the rseq_start() also set a !0 state
before reading the seq, but not sure that all is worth it.