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]

Date:   Fri, 7 Aug 2020 14:07:59 -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 7, 2020, at 1:48 PM, Peter Oskolkov posk@...k.io wrote:

> On Thu, Aug 6, 2020 at 10:37 AM Mathieu Desnoyers
> <mathieu.desnoyers@...icios.com> wrote:
>>
[...]
>> 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.
> 
> Yes, this is more about rseq; on the other hand, the high-level API/behavior
> looks closer to that membarrier, and a lot of code will be shared.
> 
> As you are the maintainer for both rseq and membarrier, this is for
> you to decide, I guess... :)

Considering that membarrier has been made extensible with the cmd
argument, and on the other hand rseq can be extended with "flags", but is
currently only about registration/unregistration, I think adding a command
to membarrier is indeed a natural approach.

I am not very fond on re-purposing the membarrier flags parameter into a
cpu number though. Maybe we should tweak the membarrier system call so it
can expect 3 arguments instead ?

  int membarrier(int cmd, int flags, int cpu);

where cpu is only used for specific commands.

One thing I find weird about Peter's patch is that it adds a
MEMBERRIER_CMD_PRIVATE_EXPEDITED_RSEQ without a corresponding
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ. Considering that
the SYNC_CORE variant already has its own register command, I
find it weird that the RSEQ counterpart does not have one.

Also, do we want to allow a RSEQ | SYNC_CORE private expedited
membarrier as well ? If that is the case, then we might want to
investigate exposing RSEQ-membarrier as a new membarrier flag
rather than as a stand-alone command.

Thanks,

Mathieu

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