On 12.08.2021 19:03, Andrew Cooper wrote:
> This was a clear oversight in the original CET work. The BUGFRAME_run_fn and
> BUGFRAME_warn paths update regs->rip without an equivlenet adjustment to the
> shadow stack, causes IRET to suffer #CP due to the mismatch.
>
> One subtle, and therefore fragile, aspect of extable_shstk_fixup() was that it
> required regs->rip to have its old value as a cross-check that the correct
> word in the shadow stack was being adjusted.
>
> Rework extable_shstk_fixup() into fixup_exception_return() which takes
> ownership of the update to both the regular and shadow stacks, ensuring that
> the regs->rip update is ordered suitably.
>
> Use the new fixup_exception_return() for BUGFRAME_run_fn and BUGFRAME_warn to
> ensure that the shadow stack is updated too.
>
> Fixes: 209fb9919b50 ("x86/extable: Adjust extable handling to be shadow stack
> compatible")
> Reported-by: Marek Marczykowski-Górecki <[email protected]>
> Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
> I'm not a massive fan of the large ifdef area. The logic could be rearranged
> to use IS_ENABLED(CONFIG_XEN_SHSTK) by indenting most of the function, but I
> can't see any way to drop the goto's, and this is certainly the least-invasive
> diff.
It's not really neat, but we've got worse code elsewhere.
I wonder whether gdb_arch_resume() and gdb_arch_write_reg() also
need some sort of similar adjustment.
Jan
