linux-kernel - Re: [RFC PATCH v9 for 4.15 01/14] 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] [day] [month] [year] [list]

Date:   Mon, 23 Oct 2017 20:44:01 +0000 (UTC)
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Ben Maurer <bmaurer@...com>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Paul Turner <pjt@...gle.com>, Andrew Hunter <ahh@...gle.com>,
        Andy Lutomirski <luto@...capital.net>,
        Dave Watson <davejwatson@...com>,
        Josh Triplett <josh@...htriplett.org>,
        Will Deacon <will.deacon@....com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andi Kleen <andi@...stfloor.org>, Chris Lameter <cl@...ux.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Russell King <linux@....linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-api <linux-api@...r.kernel.org>
Subject: Re: [RFC PATCH v9 for 4.15 01/14] Restartable sequences system call

----- On Oct 23, 2017, at 7:30 PM, Ben Maurer bmaurer@...com wrote:

>> if (!((long)ip - (long)start_ip <= (long)post_commit_offset))
>>   return 1;
> 
>> This introduces an issue here: if "ip" is lower than "start_ip", we
>> can incorrectly think we are in a critical section, when we are in
>> fact not.
> 
> This shouldn't be an issue if we used unsigned numbers. Eg if start_ip is X and
> post_commit_offset is L, then (ip - X <= L) means that if ip is less than X ip
> - X will be signed, which will become a large unsigned value.
> 
>> or to the kernel to set it back to NULL if it finds out that it is
>> preempting/delivering a signal over an instruction pointer outside
>> of the current rseq_cs start_ip/post_commit_ip range (lazy clear).
> 
> I see, lazy clear makes sense. Still, if during most execution periods the user
> code enters some rseq section (likely if rseq is used for something like
> malloc) on every context switch this code will have to be run.
> 
>> Moreover, this modification would add a subtraction on the common case
>> (ip - start_ip), and makes the ABI slightly uglier.
> 
> We could benchmark it but the subtraction should be similar in cost to the extra
> comparison but reducing the number of branches seems like it will help as well.
> FWIW GCC attempts to translate this kind of sequence to a subtract and compare:
> https://godbolt.org/g/5DGLvo.
> 
> I agree the ABI is uglier, but since we're mucking with every context switch I
> thought I'd point it out.

Thanks for following up on this. I did not initially realize the importance
of doing the unsigned comparison. I've pushed a commit in my private dev branch
implementing your suggestion.

https://github.com/compudj/linux-percpu-dev/commit/4cf8e9104636b51741c0118f2c88519e3acab7aa

Thanks!

Mathieu

> 
>> If I understand well, you are proposing to speed up .so load time by
>> means of removing relocations of pointers within rseq_cs, done by
>> making those relative to the rseq_cs address.
> 
> Yeah, I think this may be overkill as optimization.

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