Page MenuHomeFreeBSD

Introduce pmap_change_prot().
ClosedPublic

Authored by markj on Sep 22 2019, 7:51 PM.
Tags
None
Referenced Files
F81637593: D21758.diff
Fri, Apr 19, 8:00 AM
Unknown Object (File)
Feb 24 2024, 3:10 AM
Unknown Object (File)
Dec 20 2023, 1:02 AM
Unknown Object (File)
Dec 18 2023, 10:32 PM
Unknown Object (File)
Dec 18 2023, 6:56 PM
Unknown Object (File)
Dec 18 2023, 6:52 PM
Unknown Object (File)
Oct 9 2023, 11:49 AM
Unknown Object (File)
Sep 1 2023, 8:25 AM
Subscribers

Details

Summary

Generalize pmap_change_attr_locked() so that it can be used to update
mappings permissions as well. This is intended for use in the kernel
linker, both to apply protections to preloaded kernel modules (which are
not covered in kernel_map), and to ensure that the direct map is updated
when some protection changes are applied to the kernel map.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/amd64/amd64/pmap.c
8043 ↗(On Diff #62436)

Why do we need to change protection on the direct map ?

I meant to explain my intent in the description. The function is only really helpful on platforms where kernel mappings do not permit all possible access types by default, i.e., amd64. On other platforms it can simply be a no-op. If there are no objections to the approach of adding a pmap_change_prot() for use in MI code, I will write stubs for the other platforms.

sys/amd64/amd64/pmap.c
8043 ↗(On Diff #62436)

Mainly to ensure that a read-only mapping is not writeable via the direct map. We do this already for the amd64 kernel image itself, in create_pagetables().

sys/amd64/amd64/pmap.c
7790 ↗(On Diff #62436)

After you changed types to u_long, it makes no sense to have pmap_pde_props(). Might be it can exists as a macro, but I do not see much benefits.

8102 ↗(On Diff #62436)

Don't we need/want to preset PG_M ?

sys/amd64/amd64/pmap.c
7790 ↗(On Diff #62436)

Hmm, I do not see why it made sense before this diff. The only difference is the use of pd_entry_t vs. pt_entry_t. I will merge them.

8043 ↗(On Diff #62436)

Let me try to justify this approach further: initially I implemented a standalone pmap_change_prot() which only operated on the kernel map, i.e., direct map aliases were ignored. Then I realized that it was very similar to pmap_change_attr(), so I merged them. I am not sure that it is really important to modify the direct map for protection changes in the kernel map (this can be bypassed by clearing WP, after all), but conceptually it is weird to ignore the direct map alias if the intent is to restrict protections on kernel mappings, and it may help catch bugs. I also believe it will not cause excessive demotions: preloaded kernel modules are contiguous in physical memory, and modules loaded by link_elf.c will generally be backed by pages allocated from a reservation, provided that they are loaded during system startup.

8102 ↗(On Diff #62436)

I'm not sure. I think that would be incorrect for the pageable submaps. In other cases we always preset PG_M, I believe, so I can't see a case where setting PG_M here would change anything.

markj marked an inline comment as done.

Merge pmap_pte_props() and pmap_pde_props().

This revision is now accepted and ready to land.Sep 23 2019, 5:13 PM
This revision was automatically updated to reflect the committed changes.