Page MenuHomeFreeBSD

implement W^X for mmap and mprotect
AbandonedPublic

Authored by emaste on May 20 2020, 6:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 15, 2:40 AM
Unknown Object (File)
Oct 4 2024, 8:59 PM
Unknown Object (File)
Oct 1 2024, 6:42 PM
Unknown Object (File)
Oct 1 2024, 5:28 PM
Unknown Object (File)
Sep 29 2024, 2:20 AM
Unknown Object (File)
Sep 27 2024, 9:22 PM
Unknown Object (File)
Sep 26 2024, 2:06 AM
Unknown Object (File)
Sep 20 2024, 2:35 PM

Details

Reviewers
val_packett.cool
Summary

If sysctl vm.allow_wx = 0 then disallow prot with PROT_WRITE and PROT_EXECUTE both set, for mmap(2) and mprotect(2).

This is a naive implementation at the system call layer that enforces a restriction upon the application. We should move it at least one layer lower, but this can be used to identify and patch applications that create writable+executable mappings.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

emaste created this revision.

So far, the only thing that doesn't work for me with W^X is GJS..

And looks like elfctl already has a NT_FREEBSD_FCTL_WXNEEDED flag, all that's needed is to store it in imgact_elf to some process-related location that could be accessed in these vm_mmap functions.

emaste added subscribers: cem, jhb.

embiggen sysctl name as suggested by @cem and @jhb. "wx" might be taken to mean "W^X", i.e., that the sysctl enables the W xor X mode.

val_packett.cool updated this revision to Diff 81633.
val_packett.cool added a reviewer: emaste.

Added flag check (NT_FREEBSD_FCTL_WXNEEDED)

sys/vm/vm_mmap.c
110–111

Probably just bool/SYSCTL_BOOL now.

syscall is the wrong level of supporting this stuff. It should be done somewhere in vm_map.c to catch all cases of mappings. In particular, a lot of drivers can do that.

sys/vm/vm_mmap.c
111

I would name it allow_wx or allow_wx_mappings, proposed name is impossible to type without autocompletion.

emaste updated this revision to Diff 81905.
emaste edited reviewers, added: val_packett.cool; removed: emaste.
emaste removed a subscriber: markj.
emaste added a subscriber: markj.
  • add flag check (NT_FREEBSD_FCTL_WXNEEDED) from Greg V
  • rename allow_wx per kib
  • bool per cem

For mmap I think vm_mmap_object might the the right layer to do this that would catch all the cases @kib is worried about, and I think it just means moving the added lines there. mprotect() is probably fine as-is in this patch?

Have you written any tests for this? Those would be great to have and shouldn't be too hard to come up with?