Support changing the protection of preloaded kernel modules by
implementing pmap_change_prot on arm64 and calling it from
preload_protect.
Details
Details
- Reviewers
alc kib markj manu - Group Reviewers
arm64 - Commits
- rGf803dd1e24ea: Add pmap_change_prot on arm64
rGa85ce4ad7272: Add pmap_change_prot on arm64
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 41669 Build 38558: arc lint + arc unit
Event Timeline
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 6243 | ||
Comment Actions
Enabling this will limit the ability to set breakpoints or fbt probes on preloaded modules. On amd64 there is a global write-protect bit that is disabled before these operations, but on arm64 we will need something more fine-grained. Do you have plans to handle this?
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 6053 | A related observation but not strictly speaking an issue with this change: Consider a case like a large loadable kernel module, e.g., ZFS, where the code could be mapped as 2MB pages. Here, we are unconditionally demoting the mapping even if the protection change could be applied to the 2MB mapping. | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 6002 | I would make this an XXX comment. Presumably we should have ddb and dtrace temporarily modify the mapping in order for them to do their work? How do we know that ddb and dtrace always write through the direct map? | |
| 6076 | Shouldn't we try to update the direct map all at once, like amd64's version does? Otherwise, even if this function avoids demoting large mappings as Alan suggests above, we will end up demoting anyway since the direct map alias is updated a page at a time. | |
| 6247 | Isn't kernel_vm_end the appropriate upper bound there? BTW, on amd64 we avoid this by populating the radix tree with bootstrap PTPs in pmap_init(). | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 6075 | I think it violates ARM by (even if only short-term) mapping the same physical page multiple times with different cache attributes. This is explicitly marked as undefined behavior in ARM. I'm afraid that the only conforming approach is the break-before-make approach - so first break all mappings and then recreate them with new attributes. | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 6002 | I've committed D32053 that uses the DMAP region to get a RW address. I'm planning on having ddb and dtrace temporally upgrade the DMAP memory to RW if needed. | |
| 6053 | I already had a work in progress patch for this, although it still needs some polish. | |
| 6076 | It shouldn't be too bad for the DMAP as we always map it RW so wouldn't be split when changing permissions. I can have a look at adding a variant of pmap_update_entry that takes multiple entries. We could then use it to change the permissions/attributes in a single operation & only demote when needed. | |