On 2/15/22 09:50, Brian Geffon wrote:
is not a kernel thread as kernel threads will never use PKRU. It's possible that this_cpu_read_stable() on current_task (ie. get_current()) is returning an old cached value. By forcing the read with this_cpu_read() the correct task is used. Without this it's possible when switching from a kernel thread to a userspace thread that we'll still observe the PF_KTHREAD flag and never restore the PKRU. And as a result this issue only occurs when switching from a kernel thread to a userspace thread, switching from a non kernel thread works perfectly fine because all we consider in that situation is the flags from some other non kernel task and the next fpu is passed in to switch_fpu_finish().
It makes *sense* that there would be a place in the context switch code where 'current' is wonky, but I never realized this. This seems really fragile, but *also* trivially detectable.
Is the PKRU code really the only code to use 'current' in a buggy way like this?
Yes, because the remaining code in __switch_to() references the next task as next_p rather than current. Technically, it might be more correct to pass next_p to switch_fpu_finish(), what do you think? This may make sense since we're also passing the next fpu anyway.
Yeah, passing next_p instead of next_fpu makes a lot of sense to me.