[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c1ad304-61bb-4bdf-aa75-8633f3d0196c@efficios.com>
Date: Fri, 10 Jan 2025 12:02:27 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: libc-alpha <libc-alpha@...rceware.org>,
Florian Weimer <fweimer@...hat.com>, "carlos@...hat.com"
<carlos@...hat.com>, Mark Rutland <mark.rutland@....com>,
linux-kernel <linux-kernel@...r.kernel.org>, x86@...nel.org,
paulmck <paulmck@...nel.org>, Michael Jeanson <mjeanson@...icios.com>
Subject: Re: Prevent inconsistent CPU state after sequence of dlclose/dlopen
On 2025-01-10 11:54, Peter Zijlstra wrote:
> On Fri, Jan 10, 2025 at 10:55:36AM -0500, Mathieu Desnoyers wrote:
>> Hi,
>>
>> I was discussing with Mark Rutland recently, and he pointed out that a
>> sequence of dlclose/dlopen mapping new code at the same addresses in
>> multithreaded environments is an issue on ARM, and possibly on Intel/AMD
>> with the newer TLB broadcast maintenance.
>
> What is the exact race? Should not munmap() invalidate the TLBs before
> it allows overlapping mmap() to complete?
The race Mark mentioned (on ARM) is AFAIU the following scenario:
CPU 0 CPU 1
- dlopen()
- mmap PROT_EXEC @addr
- fetch insn @addr, CPU state expects unchanged insn.
- execute unrelated code
- dlclose(addr)
- munmap @addr
- dlopen()
- mmap PROT_EXEC @addr
- fetch new insn @addr. Incoherent CPU state.
>
> Any concurrent access after munmap() / before mmap() completes is UB
> anyway, no?
The problematic access happens after the second mmap. The issue is
stale CPU state.
>
>> I maintain the membarrier(2) system call, which provides a
>> MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE command for this
>> purpose. It's been there since Linux 4.16. It can be configured
>> out (CONFIG_MEMBARRIER=n), but it's enabled by default.
>>
>> Calling this after dlclose() in glibc would prevent this issue.
>>
>> Is it handled in some other way, or should we open a bugzilla
>> entry to track this ?
>
> The problem is that the membarrier() call has significant cost, and is
> only really needed if dlopen() is called right after (in the same
> location).
Or if it has any overlapping executable range.
>
> Unconditionally adding that barrier, just in case, might regress things,
> no?
Or perhaps we could add this barrier within mprotect(2) and munmap(2) in the
following cases:
- mprotect removes PROT_EXEC from a mapping,
- munmap unmaps a PROT_EXEC mapping.
Else userspace has to explicitly invoke membarrier sync-core from dlclose.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Powered by blists - more mailing lists