linux-kernel - Re: [RFC PATCH v2 02/10] lib: vdso: move call to fallback out of common code.

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: <36f1ce73-d8bc-9c46-8a2a-b6514d4a1ba0@c-s.fr>
Date:   Tue, 24 Dec 2019 12:41:41 +0100
From:   christophe leroy <christophe.leroy@....fr>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Arnd Bergmann <arnd@...db.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        LKML <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "open list:MIPS" <linux-mips@...r.kernel.org>,
        X86 ML <x86@...nel.org>
Subject: Re: [RFC PATCH v2 02/10] lib: vdso: move call to fallback out of
 common code.



Le 24/12/2019 à 03:24, Andy Lutomirski a écrit :
> On Mon, Dec 23, 2019 at 6:31 AM Christophe Leroy
> <christophe.leroy@....fr> wrote:
>>
>> On powerpc, VDSO functions and syscalls cannot be implemented in C
>> because the Linux kernel ABI requires that CR[SO] bit is set in case
>> of error and cleared when no error.
>>
>> As this cannot be done in C, C VDSO functions and syscall'based
>> fallback need a trampoline in ASM.
>>
>> By moving the fallback calls out of the common code, arches like
>> powerpc can implement both the call to C VDSO and the fallback call
>> in a single trampoline function.
> 
> Maybe the issue is that I'm not a powerpc person, but I don't
> understand this.  The common vDSO code is in C.  Presumably this means
> that you need an asm trampoline no matter what to call the C code.  Is
> the improvement that, with this change, you can have the asm
> trampoline do a single branch, so it's logically:
> 
> ret = [call the C code];
> if (ret == 0) {
>   set success bit;
> } else {
>   ret = fallback;
>   if (ret == 0)
>    set success bit;
> else
>    set failure bit;
> }

More simple than above, in fact it is:

ret = [call the C code];
if (ret == 0) {
  set success bit;
} else {
  ret = fallback [ which sets the success/failure bit];
}
return ret


> 
> return ret;
> 
> instead of:
> 
> ret = [call the C code, which includes the fallback];

C code cannot handle the success/failure bit so we need to do something 
which does:

int assembly_to_fallback()
{
	ret = [syscall the fallback]
	if (success bit set)
		return ret;
	else
		return -ret;
}

Also means going back and forth between the success bit and negative return.

> if (ret == 0)
>    set success bit;
> else
>    set failure bit;
> 
> It's not obvious to me that the former ought to be faster.
> 
>>
>> The two advantages are:
>> - No need play back and forth with CR[SO] and negative return value.
>> - No stack frame is required in VDSO C functions for the fallbacks.
> 
> How is no stack frame required?  Do you mean that the presence of the
> fallback causes worse code generation?  Can you improve the fallback
> instead?
> 

When function F1 calls function F2 (with BL insn), the link register 
(LR) is set with the return address in F1, so that at the end of F2, F2 
branches to LR (with BLR insn), that's how you return from functions.

When F2 calls function F3, the same happens, LR is set to the return of 
F3 into F2. It means that F2 has to save LR in order to be able to 
return to F1, otherwise the return address from F2 into F1 is lost.

But ... thinking about it once more, indeed fallback means doing a 
syscall, and in fact I realise that syscalls won't clobber LR, so it 
should be possible to do something. Let me try it.

Christophe