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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY4PR15MB16888F91F41A4A1D322C102CCF460@CY4PR15MB1688.namprd15.prod.outlook.com>
Date:   Mon, 23 Oct 2017 17:30:13 +0000
From:   Ben Maurer <bmaurer@...com>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.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

> 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.

> 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.