linux-kernel - Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso

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: <1b1494a8-da80-e170-78fa-48dfb3226e75@arista.com>
Date:   Sat, 27 Mar 2021 17:43:05 +0000
From:   Dmitry Safonov <dima@...sta.com>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        linux-kernel@...r.kernel.org
Cc:     Dmitry Safonov <0x7f454c46@...il.com>,
        Andrei Vagin <avagin@...il.com>,
        Andy Lutomirski <luto@...nel.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Laurent Dufour <ldufour@...ux.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Paul Mackerras <paulus@...ba.org>,
        linuxppc-dev@...ts.ozlabs.org, stable@...r.kernel.org
Subject: Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso

Hi Christophe,

On 3/27/21 5:19 PM, Christophe Leroy wrote:
[..]
>> I opportunistically Cc stable on it: I understand that usually such
>> stuff isn't a stable material, but that will allow us in CRIU have
>> one workaround less that is needed just for one release (v5.11) on
>> one platform (ppc64), which we otherwise have to maintain.
> 
> Why is that a workaround, and why for one release only ? I think the
> solution proposed by Laurentto use the aux vector AT_SYSINFO_EHDR should
> work with any past and future release.

Yeah, I guess.
Previously, (before v5.11/power) all kernels had ELF start at "[vdso]"
VMA start, now we'll have to carry the offset in the VMA. Probably, not
the worst thing, but as it will be only for v5.11 release it can break,
so needs separate testing.
Kinda life was a bit easier without this additional code.

>> I wouldn't go as far as to say that the commit 511157ab641e is ABI
>> regression as no other userspace got broken, but I'd really appreciate
>> if it gets backported to v5.11 after v5.12 is released, so as not
>> to complicate already non-simple CRIU-vdso code. Thanks!
>>
>> Cc: Andrei Vagin <avagin@...il.com>
>> Cc: Andy Lutomirski <luto@...nel.org>
>> Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>> Cc: Christophe Leroy <christophe.leroy@...roup.eu>
>> Cc: Laurent Dufour <ldufour@...ux.ibm.com>
>> Cc: Michael Ellerman <mpe@...erman.id.au>
>> Cc: Paul Mackerras <paulus@...ba.org>
>> Cc: linuxppc-dev@...ts.ozlabs.org
>> Cc: stable@...r.kernel.org # v5.11
>> [1]: https://github.com/checkpoint-restore/criu/issues/1417
>> Signed-off-by: Dmitry Safonov <dima@...sta.com>
>> Tested-by: Christophe Leroy <christophe.leroy@...roup.eu>
> 
> I tested it with sifreturn_vdso selftest and it worked, because that
> selftest doesn't involve VDSO data.

Thanks again on helping with testing it, I appreciate it!

> But if I do a mremap() on the VDSO text vma without remapping VVAR to
> keep the same distance between the two vmas, gettimeofday() crashes. The
> reason is that the code obtains the address of the data by calculating a
> fix difference from its own address with the below macro, the delta
> being resolved at link time:
> 
> .macro get_datapage ptr
>     bcl    20, 31, .+4
> 999:
>     mflr    \ptr
> #if CONFIG_PPC_PAGE_SHIFT > 14
>     addis    \ptr, \ptr, (_vdso_datapage - 999b)@ha
> #endif
>     addi    \ptr, \ptr, (_vdso_datapage - 999b)@l
> .endm
> 
> So the datapage needs to remain at the same distance from the code at
> all time.
> 
> Wondering how the other architectures do to have two independent VMAs
> and be able to move one independently of the other.

It's alright as far as I know. If userspace remaps vdso/vvar it should
be aware of this (CRIU keeps this in mind, also old vdso image is dumped
to compare on restore with the one that the host has).

Thanks,
          Dmitry