Differential D18719

Fix some issues with pmap_protect().
ClosedPublic
Actions

Authored by markj on Jan 2 2019, 6:17 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Nov 29, 5:14 PM

	Unknown Object (File)
	Oct 21 2024, 4:39 AM

	Unknown Object (File)
	Oct 1 2024, 6:26 PM

	Unknown Object (File)
	Sep 24 2024, 5:07 AM

	Unknown Object (File)
	Sep 10 2024, 8:31 PM

	Unknown Object (File)
	Sep 9 2024, 1:57 AM

	Unknown Object (File)
	Sep 7 2024, 10:23 PM

	Unknown Object (File)
	Sep 7 2024, 5:10 PM

View All 47 Files

Subscribers

Details

Reviewers

kib
alc
jhb
br
mhorne063_gmail.com

Commits

rS342734: Fix some issues with the riscv pmap_protect() implementation.

Summary

Note that pmap_protect() is only ever used to remove permissions from
existing mappings.

When clearing PTE_W, use an atomic update to the PTE; otherwise we may clobber updates done by the hardware.
Handle VM_PROT_EXECUTE.
Clear PTE_D and mark the page dirty when removing write permission from a mapping.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21808
Build 21076: arc lint + arc unit

Event Timeline

markj created this revision.Jan 2 2019, 6:17 PM

Harbormaster completed remote builds in B21808: Diff 52487.Jan 2 2019, 6:17 PM

markj added reviewers: kib, alc, jhb, br, mhorne063_gmail.com.Jan 2 2019, 6:24 PM

kib added inline comments.Jan 2 2019, 9:52 PM

sys/riscv/riscv/pmap.c
1918	I see that you changed this to atomic clear, so my comment from other review is not relevant. But what if PTE_D bit set after the pmap_load() but before atomic_clear() ? This seems to be legit and would cause missing dirty set and consequent corruption of the user data. Shouldn't you use atomic_cmpxchg() there and retry on failure ?

Fix the bug that kib pointed out.

Harbormaster completed remote builds in B21815: Diff 52498.Jan 2 2019, 11:19 PM

markj added inline comments.Jan 2 2019, 11:21 PM

sys/riscv/riscv/pmap.c
1918	Indeed, thanks. :( BTW, I used _long instead of _64 here because I believe we'll eventually want to support 32-bit kernels. I'll update the macros in D18717 in a separate commit.

kib accepted this revision.Jan 2 2019, 11:37 PM

This revision is now accepted and ready to land.Jan 2 2019, 11:37 PM

jhb accepted this revision.Jan 2 2019, 11:55 PM

jhb added inline comments.

sys/riscv/riscv/pmap.c
1874–1875	Oops.

markj mentioned this in D18717: Use regular stores for updating PTEs..Jan 3 2019, 12:21 AM

Closed by commit rS342734: Fix some issues with the riscv pmap_protect() implementation. (authored by markj). · Explain WhyJan 3 2019, 4:21 PM

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptJan 3 2019, 4:21 PM

Revision Contents
Changeset List

Path

Size

sys/

riscv/

riscv/

40 lines

Diff 52487

sys/riscv/riscv/pmap.c

Loading...