Page MenuHomeFreeBSD

riscv: Clear SUM in SSTATUS for supervisor mode exceptions.
ClosedPublic

Authored by jhb on Apr 14 2021, 4:46 PM.

Details

Summary

Previously, a page fault taken during copyin/out and related functions
would run the entire fault handler while permitting direct access to
user addresses. This could also leak across context switches (e.g. if
the page fault handler was preempted by an interrupt or slept for disk
I/O).

To fix, clear SUM in assembly after saving the original version of
SSTATUS in the supervisor mode trapframe.

Sponsored by: DARPA

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb requested review of this revision.Apr 14 2021, 4:46 PM
sys/riscv/riscv/exception.S
107–110

You should be able to li t0, SSTATUS_SUM; csrc sstatus, t0

Looks good. Just curious, does the issue manifest in practice or was it caught by inspection?

This revision is now accepted and ready to land.Apr 21 2021, 4:12 PM

Looks good. Just curious, does the issue manifest in practice or was it caught by inspection?

No evidence we've ever accidentally accessed a user address. But you do very quickly fall over with the added assertion as you take page faults in copyin/out for init. I was talking on IRC with someone who was discovering a similar issue on Linux (https://github.com/torvalds/linux/commit/285a76bb2cf51b0c74c634f2aaccdb93e1f2a359) and it made me realise this issue in FreeBSD. (If you're wondering, Armv8 doesn't have this issue, since the PAN bit gets saved to SPSR and cleared in PSTATE on traps, and x86 has the code in its trap handler to clear AC, so it's just a RISC-V issue.)