Re: [PATCH v5 07/39] x86: Add user control-protection fault handler

Borislav Petkov
2023-02-03T19:44:29Z

On Fri, Feb 03, 2023 at 07:24:08PM +0000, Edgecombe, Rick P wrote:
> The name seems better, but this is actually from the existing kernel
> IBT control protection exception code. So it seems like an separate
> change. Would you like to see it snuck into the user shadow stack
> handler, or could we leave this for future cleanups?
>
> Kees pointed out that adding to the handler and moving it in the same
> patch makes it difficult to see where the changes are. I'm splitting
> this one into two patches for the next version.

Yap, that's the right way to do it.

> I think we have to read it before we enable interrupts or use
> fpregs_lock(). So reading it before saves disabling preemption later.

So I'm a bit confused - there's that cond_local_irq_enable() which will
enable interrupts if they were enabled before.

So if they were enabled before and you reenable them here, then that
current could be the wrong one if we schedule in between, right?

IOW, shouldn't those two lines be swapped so that it says:

tsk = current;

cond_local_irq_enable(regs);

and you can be sure that tsk is always the right current which caused
the #CP? Or am I way off again?

--
Regards/Gruss,
Boris.