LWN: Comments on "Patching until the COWs come home (part 1)"

Patching until the COWs come home (part 1)

cladisch — Thu, 25 Mar 2021 07:29:38 +0000

RCU requires that all software that accesses these data structures uses certain patterns and helper functions. (This is why we have many articles about it.)

COW uses the CPU's built-in virtual memory features to make it look to userspace software as if the pages were never shared to begin with.

Patching until the COWs come home (part 1)

alison — Thu, 25 Mar 2021 04:17:34 +0000

Here is a naive question: why do we have both COW and RCU? Is there any reason fork() couldn't use RCU? Is RCU safer? RCU has extensive formal documentation but COW less so.

Patching until the COWs come home (part 1)

foom — Wed, 24 Mar 2021 01:45:12 +0000

We _almost_ have that api already: ptrace.

Minor problem being that ptrace is really awful. It's too bad the proposals to add a saner handle based version have all died off.

Patching until the COWs come home (part 1)

Cyberax — Tue, 23 Mar 2021 23:00:09 +0000

Apparently, people dislike posix_spawn. Perhaps we would grow a full replacement that would allow to create a suspended process, tweak its attributes (using file handle-based API) and then resume it.

But I feel what we might actually get is a io_uring-based API that does this using BPF.

Patching until the COWs come home (part 1)

pbonzini — Tue, 23 Mar 2021 22:45:09 +0000

On the other hand vmsplice is just a handy shortcut. AIO or any use of g_u_p() can cause the same issues.

Patching until the COWs come home (part 1)

dgc — Tue, 23 Mar 2021 21:49:45 +0000

Yup, vmsplice is the classic "solution looking for a problem to solve" interface.

FWIW, given the well known documented caveats around combining Direct IO, mmap buffers and fork to avoid data corruption and "undefined results" (see open(2) man page) because of interactions COW, I'm also not at all surprised that there are very similar issues resulting from vmsplice() taking temporary unaccounted references to user mapped pages...

-Dave.

Patching until the COWs come home (part 1)

NYKevin — Tue, 23 Mar 2021 16:12:09 +0000

Well sure, but in the (absurd) hypothetical where we're eliminating COW, the kernel would presumably need to grow an interface with capabilities similar to fork+exec, and the POSIX standard name for that interface is posix_spawn.

Patching until the COWs come home (part 1)

iabervon — Tue, 23 Mar 2021 16:00:25 +0000

The PTE has to go away because that's what munmap() is specified to do. Furthermore, the fact that it's the same process at both ends of the pipe is irrelevant to the issue, I think. It seems to me like the simple solution would be to elevate the page map count while it's in the pipe (but where else might a page be kept that a process other than parent could get it?) or use the refcount to decide if the parent is the exclusive owner (but maybe the parent has extra references?).

I think the semantics that would be clear is that, if a reference can be used to read the contents of the page, or can be converted into a reference that can be used to read the contents of the page, it counts as a map. But then you'd have to identify what needs the addition.

Patching until the COWs come home (part 1)

abatters — Tue, 23 Mar 2021 14:17:06 +0000

I considered using vfork() recently, but ultimately decided against it after encountering too many warnings about it. For example, see the history of glibc using vfork() for posix_spawn():

glibc <= 2.23:
posix_spawn() uses vfork() if POSIX_SPAWN_USEVFORK is set or if there is no cleanup expected in the child before it exec(3)s the requested file. However, this implementation of vfork() was the source of a number of bugs.

Linux glibc >= 2.24:
glibc commit 9ff72da471a509a8c19791efe469f47fa6977410
posix_spawn() switches from vfork() to clone(CLONE_VM | CLONE_VFORK) which uses a separate stack for the child. This fixes a number of vfork()-related bugs ("possible parent clobber due stack spilling"), making it possible to enable by default and ignore POSIX_SPAWN_USEVFORK.

recent non-Linux glibc
glibc commit ccfb2964726512f6669fea99a43afa714e2e6a80
POSIX_SPAWN_USEVFORK is ignored and regular fork() is always used, due to difficulties getting vfork() to work without the Linux-specific clone() semantics.

Note that using clone(CLONE_VM | CLONE_VFORK) safely requires blocking all signals, including NPTL-internal signals. But the glibc wrappers don't let you block NPTL-internal signals, making it much more difficult to do outside of glibc. See the glibc implementation for all the gory details.

Patching until the COWs come home (part 1)

gerdesj — Tue, 23 Mar 2021 12:37:35 +0000

"But, as the Project Zero issue mentions, there are environments, such as Android, where each process is forked from a zygote process without a subsequent exec(), for performance reasons. That could lead to a situation that looks a lot like the PoC exploit for this bug."

Patching until the COWs come home (part 1)

joib — Tue, 23 Mar 2021 11:01:18 +0000

A couple of years ago we discussed this paper, which argues that fork() is fundamentally the wrong primitive to build OS process management around: https://lwn.net/Articles/785430/

Patching until the COWs come home (part 1)

excors — Tue, 23 Mar 2021 10:28:40 +0000

> if you had just munmapped it, it would be really weird for a subsequent mmap of the same address and size to fail. Userspace might assume that it doesn't need to check the error code for mmap in that case (or it might not have suitable recovery code, and just call abort(3)).

That reminds me of an old bug: About a decade ago, Firefox (using code from jemalloc) would try to do a large aligned allocation like "p = mmap(NULL, size*2); munmap(p); p = mmap(round_up(p, alignment), size);" i.e. using the first mmap+munmap to discover a large-enough hole in the address space, then allocating at a correctly-aligned address within that hole. If the second mmap didn't return the address that was requested, there must have been a race condition with another thread that allocated in the same hole, so it would loop around and try again and hope for better luck next time.

That worked okay until it ran on kernels with a security feature that randomised mmap and entirely ignored the address parameter (which is technically okay since it's defined as just a hint, not a requirement), so the code got stuck in an infinite loop.

That was fixed ages ago, but it does seem plausible that some userspace code may still make similarly unwise assumptions.

Patching until the COWs come home (part 1)

pbonzini — Tue, 23 Mar 2021 10:08:13 +0000

posix_spawn() is not a single system call (technically both fork() and vfork() are wrappers around clone(2), but at least the latter is a single system call).

Patching until the COWs come home (part 1)

pbonzini — Tue, 23 Mar 2021 10:06:50 +0000

Based on a quick Debian Code Search query:

* The fio benchmarking tool supports it

* openssl uses it with AF_ALG (which is also a bit on the obscure side), and so does libkcapi (a library for the kernel crypto API)

* VLC is an interesting one, it uses vmsplice to dump the decompressed output of gzip/bzip2/xz into memory (probably because the rest of the program prefers to work with memory-mapped buffers?)

* FUSE also uses it

Also, Samba doesn't use vmsplice but it uses splice to implement the opposite of sendfile (read from a socket into a file).

Patching until the COWs come home (part 1)

NYKevin — Tue, 23 Mar 2021 08:42:23 +0000

In principle yes, but then you have other problems.

For example, mmap can be used to allocate memory at a fixed address. It can be difficult to tell whether any given address is suitable (because other pages etc. might be in the way), but if you had just munmapped it, it would be really weird for a subsequent mmap of the same address and size to fail. Userspace might assume that it doesn't need to check the error code for mmap in that case (or it might not have suitable recovery code, and just call abort(3)).

So now your solution needs to accommodate stacking a new page on top of the hidden page, or relocating the hidden page, either of which is nontrivial. That's not even mentioning the fact that you need to teach vmsplice to track per-process references in the same way as pages do, without that page actually existing in the userspace page map. These are all rather difficult problems to solve, and this is a security issue, so solving hard problems is not the ideal form of a fix. Throwing in a simple COW break is a much more straightforward solution (but, as the story alludes, there was presumably some complication which they failed to account for).

Patching until the COWs come home (part 1)

Sesse — Tue, 23 Mar 2021 08:40:04 +0000

Do any applications actually use vmsplice() at all? I remember making a prototype for a minimal HTTP server (it only ever existed to serve two static responses), and it helped a few percent, but was never put in production. It seems to have caused a fair bit of security headache over the years…

Patching until the COWs come home (part 1)

PengZheng — Tue, 23 Mar 2021 03:37:43 +0000

As a non-kernel developer, I'm curious:

In retrospect, is this COW breaking fundamentally wrong?
Why not treat vmsplice reference "as references"? Is is possible to make the page inaccessible to the userspace process after munmap while still keep the kernel's reference?

Patching until the COWs come home (part 1)

vbabka — Tue, 23 Mar 2021 00:20:21 +0000

> The child still has access to the page, so surely it should still be marked as having access to that page as long as it does indeed have a reference to it, rather than pretending it doesn't have access to the page just because it's been munmap'd.

I'm not sure I understand your point, but that problem is prevented by the commit. When the child wants to take that extra reference (for vmsplice()) it gets a copy instead of the page shared with the parent. Afterwards both the page tables of the child and the reference held by the pipe point to this new copy, and the access to parent's page is lost.

> Just because the page has been munmap'd doesn't mean that the process can't read from it, so why is the page table entry removed?

That's simply the semantic of munmap() - it has to adjust the VMA tree and zap page table entries so that the munmapped range is no longer represented there. Then if the process tries to read/write to an address within the area, it segfaults. We can't leave the page table entry there just because another reference exists. The read from the pipe doesn't go through these page tables.

Patching until the COWs come home (part 1)

milesrout — Mon, 22 Mar 2021 23:42:24 +0000

Naïve non-kernel-developer perspective: this seems _intuitively_ like the wrong solution to the problem. The child still has access to the page, so surely it should still be marked as having access to that page as long as it does indeed have a reference to it, rather than pretending it doesn't have access to the page just because it's been munmap'd. Just because the page has been munmap'd doesn't mean that the process can't read from it, so why is the page table entry removed? I assume there's a very good reason why it's done this way, though. Could someone clear up my misunderstanding?

For what it's worth, this all reminds me of the SCM_RIGHTS/io_uring issue (https://lwn.net/Articles/779472/).

Patching until the COWs come home (part 1)

NYKevin — Mon, 22 Mar 2021 23:09:48 +0000

If you're going to break API compatibility anyway,* then you might as well just tell people to use vfork() or posix_spawn(), as those are already mature and well-understood interfaces, and the latter is even portable.

Incidentally, while researching this comment, I stumbled across this tidbit from clone(2):

> In contrast to the glibc wrapper, the raw clone() system call
> accepts NULL as a stack argument (and clone3() likewise allows
> cl_args.stack to be NULL). In this case, the child uses a
> duplicate of the parent's stack. (Copy-on-write semantics ensure
> that the child gets separate copies of stack pages when either
> process modifies the stack.) In this case, for correct
> operation, the CLONE_VM option should not be specified. (If the
> child shares the parent's memory because of the use of the
> CLONE_VM flag, then no copy-on-write duplication occurs and chaos
> is likely to result.)

That sounds like it would be fun to debug. Now I'm wondering if any developers have decided that they "need to" bypass glibc and pull this sort of chicanery on any of my systems... (I'm an SRE, so if it broke in production, it would be my problem to fix it). "Fortunately," most of the bugs I've seen have tended to be higher-level than this, but it's still a bit frightening that the kernel will just let you do something like that.

* Which is obviously not going to happen given the kernel's fanatical devotion to not breaking userspace, but let's pretend for a moment.

Patching until the COWs come home (part 1)

Cyberax — Mon, 22 Mar 2021 20:52:47 +0000

Over the years I found that COWs are just bad and it's best to avoid them altogether.

Can we already use io_uring to spawn new processes?

Patching until the COWs come home (part 1)

nix — Mon, 22 Mar 2021 19:22:27 +0000

It's definitely a problem because it doesn't match the user's mental model of how fork() is supposed to work. It's clear that either COW must be broken in this case or a mapping must be retained (or the refcounts split into per-mm versions, which seems likely to be far more expensive). The conceptually ideal approach would have everything act just like normal data, i.e. recognise things like vmsplice references *as* references so you don't need to specially break COW early for them -- but this seems likely to be viciously complex and of only minor benefit. Of course, hunting down every single way a reference can be taken by a child and arranging to COW-break on all of them seems likely to be a nightmarish game of whack-a-mole too...

Patching until the COWs come home (part 1)

tux3 — Mon, 22 Mar 2021 18:55:47 +0000

What a cliffhanger!
I certainly am glad that project zero is there to root out these bugs (pun intended).