linux-kernel - Re: [PATCH 1/2] membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON

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: <1668913120.1621.1596735425601.JavaMail.zimbra@efficios.com>
Date:   Thu, 6 Aug 2020 13:37:05 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Peter Oskolkov <posk@...k.io>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Peter Oskolkov <posk@...gle.com>, paulmck <paulmck@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Paul Turner <pjt@...gle.com>,
        Chris Kennelly <ckennelly@...gle.com>
Subject: Re: [PATCH 1/2] membarrier: add
 MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU

----- On Aug 6, 2020, at 1:07 PM, Peter Oskolkov posk@...k.io wrote:

> On Thu, Aug 6, 2020 at 6:48 AM <peterz@...radead.org> wrote:
>>
>> On Wed, Aug 05, 2020 at 05:08:58PM -0700, Peter Oskolkov wrote:
>>
>> Thanks for the Cc!
> 
> Always a pleasure!
> 
> (Sorry, included only membarrier maintainers in v1; in v2 included
> both membarrier and rseq maintainers).
> 
>>
>> > + * @MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU:
>> > + *                          If a thread belonging to the current process
>> > + *                          is currently in an RSEQ critical section on the
>> > + *                          CPU identified by flags parameter, restart it.
>> > + *                          @flags: if @flags >= 0, identifies the CPU to
>> > + *                                  restart RSEQ CS on; if == -1, restarts
>> > + *                                  RSEQ CSs on all CPUs.
>>
>> > +     } else if (cpu_id == -1) {
>> > +             on_each_cpu(membarrier_rseq_ipi,
>> > +                         current->group_leader, true);
>>
>> This is an unpriv IPI the world. That's a big no-no.
> 
> removed in v2.

I don't think the feature must be removed, but its implementation needs adjustment.

How about we simply piggy-back on the membarrier schemes we already have, and
implement:

membarrier_register_private_expedited(MEMBARRIER_FLAG_RSEQ)
membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ)

All the logic is there to prevent sending IPIs to runqueues which are not running
threads associated with the same mm. Considering that preemption does an rseq abort,
running a thread belonging to a different mm should mean that this CPU is not
currently executing an rseq critical section, or if it was, it has already been
aborted, so it is quiescent.

Then you'll probably want to change membarrier_private_expedited so it takes an
extra "cpu" argument. If cpu=-1, iterate on all runqueues like we currently do.
If cpu >= 0, only IPI that CPU if the thread currently running has the same mm.

Also, should this belong to the membarrier or the rseq system call ? It just
looks like the membarrier happens to implement very similar things for barriers,
but arguably this is really about rseq. I wonder if we should expose this through
rseq instead, even if we end up using membarrier code.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com