Page MenuHomeFreeBSD

Implement INHERIT_ZERO for minherit(2).
ClosedPublic

Authored by delphij on Jul 17 2014, 2:32 AM.
Tags
None
Referenced Files
F102052646: D427.id740.diff
Thu, Nov 7, 12:46 AM
Unknown Object (File)
Wed, Oct 16, 7:41 AM
Unknown Object (File)
Thu, Oct 10, 6:37 PM
Unknown Object (File)
Sep 27 2024, 10:04 PM
Unknown Object (File)
Sep 24 2024, 4:08 AM
Unknown Object (File)
Sep 20 2024, 5:46 AM
Unknown Object (File)
Sep 19 2024, 9:19 AM
Unknown Object (File)
Sep 17 2024, 6:31 AM

Details

Summary

INHERIT_ZERO is an OpenBSD feature. When a page is marked as such, it would
be zeroed upon fork(). This would be used in new arc4random(3) functions.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

delphij retitled this revision from to Implement INHERIT_ZERO for minherit(2)..
delphij updated this object.
delphij edited the test plan for this revision. (Show Details)
delphij added reviewers: kib, jhb.

The unobvious consequence of this patch is that not only VM_INHERIT_ZERO changes map entry in the child, but it also modifies behaviour of the fork() WRT the parent entry. For VM_INHERIT_COPY, both parent and child get private shadow objects inserted above the backing object for the entry. For VM_INHERIT_ZERO, child gets fresh empty object, while parent is kept without the shadow. In other words, VM_INHERIT_COPY made the parent also COW, while VM_INHERIT_ZERO implicitely shares.

I am not sure if this is important, but it is subtle.

sys/vm/vm_map.c
3355

This comment is definitely not true. I suppose that the code was copied from the VM_INHERIT_COPY case. It might be more reasonable to not do *new_entry = *old_entry assignment at all, since the only copied fields are start/end and (partially) flags.

3366

This is not the correct test. The entry charge may be already migrated to the backing object. You should use ENTRY_CHARGED() to see if the entry is accounted for swap use.

Looking at it some more, I think now that the test is not needed at all. New entry is the anonymous memory, virtually allocated at the fork time. The charge must be applied to the child' map entry unconditionally, it does not matter what kind of memory backed the same range in the parent process.

delphij edited edge metadata.

Attempt to fix most problems raised by kib@:

  • Fixed comments;
  • Limit new_entry copying by explicitly copying fields we need;
  • Unconditionally charge process because we are (actually) allocating new pages.

I don't see anything wrong, but I don't promise to understand all the details of setting up the new vm map entry correctly.

emaste added inline comments.
sys/sys/mman.h
46

Can we make this MAP_INHERIT_ZERO (and provide MAP_ versions of the other ones, too)? OpenBSD and NetBSD use MAP_*, and there was discussion long ago about using the same #defines in FreeBSD (from before user-visible constants appeared in mman.h)

It would be nice to avoid needing compatibility #defines or #ifdef FreeBSD tests in all of the consumers.

Anyway, it is relatively easy to implement syscall-less getpid(),
and simultaneously implement something which I called forkdepth(3).

So you'd use as the reseed necessary test getpid() != last_pid || forkdepth() != last_forkdepth?

sys/vm/vm_map.c
3364

Hm, you need also avail_ssize, adh_free and max_free copied as well. Does it make sense to set stack to VM_INHERIT_ZERO ?

Copy avail_ssize, adj_free and max_free.

(I don't think it makes sense to INHERIT_ZERO on stack though).

This revision was automatically updated to reflect the committed changes.
wblock added inline comments.
head/lib/libc/sys/minherit.2
95 ↗(On Diff #26260)

Change from passive to active and edit a bit:

Cause the address space in question to be mapped as new
anonymous pages which are initialized to all zero bytes in the
child process.
139 ↗(On Diff #26260)

This "The" is not necessary.