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.
Details
Details
- Reviewers
kib jhb - Group Reviewers
manpages - Commits
- rS319278: MFC r315272, r315370
rS319277: MFC r315272, r315370
rS315272: Implement INHERIT_ZERO for minherit(2).
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 8040 Build 8225: CI src build Jenkins Build 8224: arc lint + arc unit
Event Timeline
Comment Actions
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 | ||
|---|---|---|
| 3485 | 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. | |
| 3496 | 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. | |
Comment Actions
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.
Comment Actions
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.
Comment Actions
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.
| 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. | |
Comment Actions
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 | ||
|---|---|---|
| 3494 | 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 ? | |
Comment Actions
Copy avail_ssize, adj_free and max_free.
(I don't think it makes sense to INHERIT_ZERO on stack though).