Remove execute permissions from mappings in exec_map and pipe_map.
ClosedPublic
Actions

Authored by markj on Nov 3 2018, 5:12 PM.

Details

Reviewers

alc
kib
jhb

Commits

rS340205: Avoid specifying VM_PROT_EXECUTE in mappings from pipe_map and exec_map.

Summary

They are not needed. I considered also adding prot and maxprot
parameters to kmem_suballoc(), but that doesn't appear to have any
effect on submaps, so I didn't bother.

Reported by: jhb

Test Plan

I used kgdb to examine PTEs in the range managed by the two
submaps and verified that NX was present where it wasn't before.

I audited uses of exec_map and pipe_map in the kernel (exec_map
has some uses in some exotic image activators, though most
uses look like they could be replaced with an allocation of a single
wired page).

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 20612
Build 20032: arc lint + arc unit

Event Timeline

markj created this revision.Nov 3 2018, 5:12 PM

Harbormaster completed remote builds in B20612: Diff 49974.Nov 3 2018, 5:12 PM

markj edited the summary of this revision. (Show Details)Nov 3 2018, 5:22 PM

markj edited the test plan for this revision. (Show Details)

markj added reviewers: alc, kib, jhb.

markj marked an inline comment as done.Nov 3 2018, 5:26 PM

markj added inline comments.

sys/vm/vm_kern.c
628	This should be "maxprot".

"max" -> "maxprot"

Harbormaster completed remote builds in B20613: Diff 49975.Nov 3 2018, 5:48 PM

kib added inline comments.Nov 4 2018, 5:08 PM

sys/kern/kern_exec.c
1350–1351	This is the only use of the kmap_alloc_wait() function that I am able to find. Why do you think it is useful to add prot and maxprot args to it, instead of hardcoding VM_PROT_RW into the vm_map_insert() call from the function ?

markj marked an inline comment as done.Nov 4 2018, 5:15 PM

markj added inline comments.

sys/kern/kern_exec.c
1350–1351	kmap_alloc_wait() is a simple wrapper of vm_map_insert(); it seemed a bit strange to me to bake this policy into the implementation. I don't think it's particularly useful to have the extra arguments, though, so I will change kmap_alloc_wait() instead if you prefer.

kib accepted this revision.Nov 4 2018, 5:24 PM

kib added inline comments.

sys/kern/kern_exec.c
1350–1351	The current code hardcodes the policy as well. I was more curious about implied usefulness of the prot/maxprot arguments.

This revision is now accepted and ready to land.Nov 4 2018, 5:24 PM

jhb added inline comments.Nov 5 2018, 6:07 PM

sys/kern/kern_exec.c
1350–1351	I do probably find it a bit odd to provide both arguments. I would probably lean towards either hardcoding a RW policy or providing a single 'prot' argument that is used as both 'prot' and 'maxprot', but that is a minor detail.