Page MenuHomeFreeBSD

Control implicit PROT_MAX() using proccontrol(1) and the FreeBSD note feature bit.
ClosedPublic

Authored by kib on Jun 28 2019, 6:16 PM.
Tags
None
Referenced Files
F118730896: D20795.diff
Sun, Jun 1, 7:53 AM
Unknown Object (File)
Fri, May 30, 10:59 PM
Unknown Object (File)
Wed, May 28, 3:19 PM
Unknown Object (File)
Tue, May 27, 5:48 AM
Unknown Object (File)
May 1 2025, 2:18 AM
Unknown Object (File)
Apr 18 2025, 8:24 PM
Unknown Object (File)
Apr 17 2025, 11:03 PM
Unknown Object (File)
Apr 15 2025, 6:38 PM
Subscribers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Overall looks reasonable, a few small alpha ordering nits.

sys/compat/freebsd32/freebsd32_misc.c
3337 ↗(On Diff #59163)

alpha order

sys/vm/vm_mmap.c
59 ↗(On Diff #59163)

can this go in alpha order, other misordered headers notwithstanding?

usr.bin/proccontrol/proccontrol.c
76 ↗(On Diff #59163)

alpha order

Suggested rewording of a sentence.

lib/libc/sys/procctl.2
129 ↗(On Diff #59163)

The image flag might still prevent the enablement.

kib marked 4 inline comments as done.Jun 28 2019, 7:31 PM
kib added inline comments.
sys/vm/vm_mmap.c
59 ↗(On Diff #59163)

Not sure what would be the right place then. Moved before filedesc.h.

lib/libc/sys/procctl.2
119 ↗(On Diff #59163)

and .Xr mprotect 2

128 ↗(On Diff #59163)

It would be nice to reference the sysctl here, "disabled system-wide by vm.imply_prot_max"

131 ↗(On Diff #59163)

Should be "be disabled."

133 ↗(On Diff #59163)

Extra _ between PROT and MAX.

136 ↗(On Diff #59163)

"Use the system-wide ..."

kib marked an inline comment as done.

Man page update and minor code rearrangement.

This revision is now accepted and ready to land.Jul 2 2019, 3:00 PM
lib/libc/sys/procctl.2
119 ↗(On Diff #59163)

We don't imply PROT_MAX on mprotect at this time. One could make a case for it, but we're never tested it and it's not as clear a case IMO.

sys/sys/syscallsubr.h
178 ↗(On Diff #59163)

It seems a little odd to me that this is here and not in some sys/vm/* header, but don't have a strong opinion.