Re: Prevent inconsistent CPU state after sequence of dlclose/dlopen

[Mathieu Desnoyers]

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