Re: [PATCH v3 10/12] um: remove force_flush_all from fork_handler

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2024-07-03 at 11:45 +0200, Johannes Berg wrote:
> On Fri, 2024-05-24 at 23:37 +0200, benjamin@sipsolutions.net wrote:
> > From: Benjamin Berg <benjamin.berg@intel.com>
> > 
> > There should be no need for this.
> 
> "should" ;-)
> 
> This breaks things if glibc enables rseq. That might even be already
> broken in the sense that it might corrupt memory that's put at the same
> place the rseq was installed? But it at least it appears to still work
> without this patch...

Uh, well, it's not quite right what I'm saying ...

Courtesy of Benjamin, currently we have, in userspace memory:


|-- normal address space --|-- empty --|-- stub --|-- empty ...
                           ^ TASK_SIZE
                                       ^ STUB_START

now it appears that - at least in 32-bit - the rseq memory is between
TASK_SIZE and STUB_START, so the

> +	/* Ensure the new MM is clean and nothing unwanted is mapped */
> +	unmap(new_id, 0, STUB_START);

will unmap the memory glibc set up for rseq, and thus immediately lead
to a SIGSEGV when rseq will try to use it, but that happens immediately.

Thus, I see the crash.

If we unmap there only to TASK_SIZE, which is less clean but pretty much
OK since we only use the memory < TASK_SIZE, then this problem goes away
(and Benjamin will resend the patches with that, for now, until it all
goes away with the execve patch.)

If rseq memory was to overlap the "-- stub --" area, then we'd get the
stub corrupted and crash, but I haven't observed that (yet anyway.)
Although perhaps depending on how the corruption works it's not even too
likely.

In any case, it's either not really an issue (rseq memory remains mapped
in the "empty" blocks), or already causing stub corruption, so this
patchset doesn't really (need to) change it, and the later execve()
change will clean it all up properly.

johannes