Page MenuHomeFreeBSD

procctl(2) and proccontrol(1): add W^X control
ClosedPublic

Authored by kib on Sep 2 2021, 3:26 AM.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib requested review of this revision.EditedSep 2 2021, 3:26 AM
kib created this revision.

I checked that go binaries that have no PT_GNU_STACK segment and no elfctl note to disable wxorx for the image, do start with kern.elf64.allow_wx=0 and 'proccontrol -m wxorx -s disable <go program>'

lib/libc/sys/procctl.2
618 ↗(On Diff #94525)
622 ↗(On Diff #94525)
629 ↗(On Diff #94525)
643 ↗(On Diff #94525)
644 ↗(On Diff #94525)
648 ↗(On Diff #94525)
653 ↗(On Diff #94525)
654 ↗(On Diff #94525)
708 ↗(On Diff #94525)
711 ↗(On Diff #94525)

I would perhaps s/like JIT// and add another sentence along the lines of, "This may be legitimately required by some programs, such as JIT compilers."

sys/kern/kern_procctl.c
605 ↗(On Diff #94525)

I think you need a P_WEXIT check here as you have below.

643 ↗(On Diff #94525)

What's the point of unlocking the proc at all?

kib marked 12 inline comments as done.Sep 2 2021, 6:26 PM
kib added inline comments.
sys/kern/kern_procctl.c
643 ↗(On Diff #94525)

vmspace_acquire_ref() potentially results in the call to vmspace_free()->vmspace_dofree()->VM locking (e.g. sleepable vm map lock take)

kib marked an inline comment as done.

Regularize prologue in wxorx_status/ctl, check P_WEXIT and assert process lock.
Man page language fixes.

markj added inline comments.
lib/libc/sys/procctl.2
608 ↗(On Diff #94568)
This revision is now accepted and ready to land.Sep 10 2021, 1:13 PM
kib marked an inline comment as done.Sep 10 2021, 1:25 PM
lib/libc/sys/procctl.2
606 ↗(On Diff #94568)

I'm concerned by the different sense of the two flags, WXORX being enabled is equivalent to allow_wx=0.

617 ↗(On Diff #94568)

this is why I desire positive sense flags (and why sysctl(9) recommends positive sense): this *_DISABLE flag enables something, which can be confusing

622 ↗(On Diff #94568)

and *_ENABLE_* prevents something

lib/libc/sys/procctl.2
617 ↗(On Diff #94568)

This problem exists no matter what, it's just a question of what you're disabling: the mitigation, or the ability to create writeable, executable mappings. PROC_WXORX_DISABLE and PROC_WXORX_ENABLE_ON_EXEC both clearly refer to the mitigation.

A more explicit naming scheme would be to have PROC_PERMIT_WX_MAPPINGS and PROC_PERMIT_WX_MAPPINGS_ON_EXEC or something similar. That is, instead of referring to the mitigation, refer to the underlying capability. I think I slightly prefer that approach since it's a bit clearer and agrees with the sysctl.

kib marked 4 inline comments as done.

Rename to PROC_WXMAP, PROC_WX_MAPPINGS_PERMIT, and PROC_WX_MAPPINGS_DISALLOW_EXEC, as suggested by Markj.

This revision now requires review to proceed.Sep 16 2021, 7:25 PM
markj added inline comments.
usr.bin/proccontrol/proccontrol.1
75 ↗(On Diff #95265)
This revision is now accepted and ready to land.Sep 16 2021, 7:56 PM
kib marked an inline comment as done.Sep 16 2021, 8:11 PM
This revision was automatically updated to reflect the committed changes.
kib added a commit: R10:1349891a0eed: Style.