From: Michael Ellerman <mpe@ellerman.id.au>
To: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
nathanl@linux.ibm.com, linux-arch <linux-arch@vger.kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
open list <linux-kernel@vger.kernel.org>,
Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
linuxppc-dev@lists.ozlabs.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
Date: Fri, 28 Aug 2020 12:14:28 +1000 [thread overview]
Message-ID: <87a6yf34aj.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <CAJwJo6ZANqYkSHbQ+3b+Fi_VT80MtrzEV5yreQAWx-L8j8x2zA@mail.gmail.com>
Dmitry Safonov <0x7f454c46@gmail.com> writes:
> Hello,
>
> On Wed, 26 Aug 2020 at 15:39, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> [..]
>> > arch_remap() gets replaced by vdso_remap()
>> >
>> > For arch_unmap(), I'm wondering how/what other architectures do, because
>> > powerpc seems to be the only one to erase the vdso context pointer when
>> > unmapping the vdso.
>>
>> Yeah. The original unmap/remap stuff was added for CRIU, which I thought
>> people tested on other architectures (more than powerpc even).
>>
>> Possibly no one really cares about vdso unmap though, vs just moving the
>> vdso.
>>
>> We added a test for vdso unmap recently because it happened to trigger a
>> KAUP failure, and someone actually hit it & reported it.
>
> You right, CRIU cares much more about moving vDSO.
> It's done for each restoree and as on most setups vDSO is premapped and
> used by the application - it's actively tested.
> Speaking about vDSO unmap - that's concerning only for heterogeneous C/R,
> i.e when an application is migrated from a system that uses vDSO to the one
> which doesn't - it's much rare scenario.
> (for arm it's !CONFIG_VDSO, for x86 it's `vdso=0` boot parameter)
Ah OK that explains it.
The case we hit of VDSO unmapping was some strange "library OS" thing
which had explicitly unmapped the VDSO, so also very rare.
> Looking at the code, it seems quite easy to provide/maintain .close() for
> vm_special_mapping. A bit harder to add a test from CRIU side
> (as glibc won't know on restore that it can't use vdso anymore),
> but totally not impossible.
>
>> Running that test on arm64 segfaults:
>>
>> # ./sigreturn_vdso
>> VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
>> Signal delivered OK with VDSO mapped
>> VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
>> Signal delivered OK with VDSO moved
>> Unmapped VDSO
>> Remapped the stack executable
>> [ 48.556191] potentially unexpected fatal signal 11.
>> [ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
>> [ 48.556990] Hardware name: linux,dummy-virt (DT)
>> [ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
>> [ 48.557475] pc : 0000ffff8191a7bc
>> [ 48.557603] lr : 0000ffff8191a7bc
>> [ 48.557697] sp : 0000ffffc13c9e90
>> [ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
>> [ 48.558201] x27: 0000000000000000 x26: 0000000000000000
>> [ 48.558337] x25: 0000000000000000 x24: 0000000000000000
>> [ 48.558754] x23: 0000000000000000 x22: 0000000000000000
>> [ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
>> [ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
>> [ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
>> [ 48.559312] x15: 0000000000000000 x14: 000000000000001c
>> [ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
>> [ 48.559625] x11: 0000000000000003 x10: 0101010101010101
>> [ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
>> [ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
>> [ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
>> [ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
>> [ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
>> Segmentation fault
>> #
>>
>> So I think we need to keep the unmap hook. Maybe it should be handled by
>> the special_mapping stuff generically.
>
> I'll cook a patch for vm_special_mapping if you don't mind :-)
That would be great, thanks!
cheers
next prev parent reply other threads:[~2020-08-28 2:14 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 13:16 [PATCH v8 0/8] powerpc: switch VDSO to C implementation Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 1/8] powerpc/vdso64: Switch from __get_datapage() to get_datapage inline macro Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage() Christophe Leroy
2020-07-16 2:59 ` Michael Ellerman
2020-08-04 11:17 ` Christophe Leroy
2020-08-25 14:15 ` Christophe Leroy
2020-08-26 13:58 ` Michael Ellerman
2020-08-27 20:34 ` Dmitry Safonov
2020-08-28 2:14 ` Michael Ellerman [this message]
2020-09-21 11:26 ` Will Deacon
2020-09-27 7:43 ` Christophe Leroy
2020-09-28 15:08 ` Dmitry Safonov
2020-10-23 11:22 ` Christophe Leroy
2020-10-23 11:25 ` Will Deacon
2020-10-23 11:57 ` Christophe Leroy
2020-10-23 13:29 ` Dmitry Safonov
2020-04-28 13:16 ` [PATCH v8 3/8] powerpc/vdso: Remove unused \tmp param in __get_datapage() Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 4/8] powerpc/processor: Move cpu_relax() into asm/vdso/processor.h Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation Christophe Leroy
2020-07-15 1:04 ` Michael Ellerman
2020-07-15 18:47 ` Christophe Leroy
2020-07-16 23:18 ` Tulio Magno Quites Machado Filho
2020-08-04 11:14 ` Christophe Leroy
2020-08-05 6:24 ` Michael Ellerman
2020-08-05 13:35 ` Segher Boessenkool
2020-08-06 2:03 ` Michael Ellerman
2020-08-06 18:33 ` Segher Boessenkool
2020-08-07 2:44 ` Michael Ellerman
2020-04-28 13:16 ` [PATCH v8 6/8] powerpc/vdso: Switch " Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 7/8] lib/vdso: force inlining of __cvdso_clock_gettime_common() Christophe Leroy
2020-04-28 13:16 ` [PATCH v8 8/8] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32 Christophe Leroy
2020-04-28 16:05 ` Arnd Bergmann
2020-05-09 15:54 ` Christophe Leroy
2020-05-09 18:48 ` Christophe Leroy
2020-05-29 18:56 ` [PATCH v8 0/8] powerpc: switch VDSO to C implementation Christophe Leroy
2020-06-03 10:04 ` Michael Ellerman
2020-07-16 12:55 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a6yf34aj.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=0x7f454c46@gmail.com \
--cc=arnd@arndb.de \
--cc=benh@kernel.crashing.org \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luto@kernel.org \
--cc=nathanl@linux.ibm.com \
--cc=paulus@samba.org \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).