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)
Feb 12 2024, 10:23 AM
Unknown Object (File)
Feb 4 2024, 2:13 PM
Unknown Object (File)
Dec 20 2023, 6:05 AM
Unknown Object (File)
Dec 14 2023, 11:29 AM
Unknown Object (File)
Dec 10 2023, 10:44 PM
Unknown Object (File)
Oct 10 2023, 8:42 PM
Unknown Object (File)
Oct 5 2023, 7:36 PM
Unknown Object (File)
Sep 16 2023, 5:05 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

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?