Check for invalid sstatus values in set_mcontext().

[jhb]

Summary

Previously, this check was only in sys_sigreturn() which meant that
user applications could write invalid values to the register via
setcontext() or swapcontext().

Test Plan

• booted riscv64 in spike

[mhorne]

sys/riscv/riscv/machdep.c

375 ↗

(On Diff #66874)

I don't think this comment accurately describes what is being checked here. The SPP bit in sstatus is used to indicate the privilege mode in which the hart was executing before taking a trap. In this case we would expect to be coming from user mode, so we check that SPP == 0.

To check that interrupts haven't been tampered with, we would expect to see that the SPIE bit is set, indicating that supervisor interrupts were enabled before the trap was taken, which as the comment state we expect to always be true.

I don't think there would be any harm in checking both of these conditions.

[jhb]

sys/riscv/riscv/machdep.c

375 ↗

(On Diff #66874)

I had merely preserved the comment in this case, but I think you are saying to just add an SPIE check explicitly and then it will match the comment? It's probably best to do that as a followup I think if so.

[mhorne]

sys/riscv/riscv/machdep.c

375 ↗

(On Diff #66874)

Yes, that is what I meant. Doing it as a follow-up would be great.

[jhb]

sys/riscv/riscv/machdep.c

375 ↗

(On Diff #66874)

Actually, I wonder if we should instead be treating this field a bit differently and either ignoring it altogether (so it's effectively a read-only field for userland) or only allowing userland to set the bits in ustatus and preserving all non-ustatus bits in tf_sstatus.