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

Edgecombe, Rick P
2023-02-03T23:01:58Z

On Fri, 2023-02-03 at 20:44 +0100, Borislav Petkov wrote:
> > 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?

Since this path is only for exceptions coming from userspace, I think
it should be valid either way. It can't be during a task switch.
I can swap the lines if it looks odd, but unless I'm wrong about the
'current' validity I think it's negligibly better as is because it is
preemptible for as long as possible.