Messages in this thread | | | Subject | Re: [patch V3 3/6] x86/process: Check PF_KTHREAD and not current->mm for kernel threads | From | Andy Lutomirski <> | Date | Thu, 10 Jun 2021 18:04:30 -0700 |
| |
On 6/10/21 1:54 PM, Thomas Gleixner wrote: > On Thu, Jun 10 2021 at 10:10, Andy Lutomirski wrote: > >> On Tue, Jun 8, 2021, at 7:36 AM, Thomas Gleixner wrote: >>> switch_fpu_finish() checks current->mm as indicator for kernel threads. >>> That's wrong because kernel threads can temporarily use a mm of a user >>> process via kthread_use_mm(). >>> >>> Check the task flags for PF_KTHREAD instead. >>> >>> Fixes: 0cecca9d03c9 ("x86/fpu: Eager switch PKRU state") >>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >>> Cc: Rik van Riel <riel@surriel.com> >>> Cc: stable@vger.kernel.org >>> --- >>> arch/x86/include/asm/fpu/internal.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> --- a/arch/x86/include/asm/fpu/internal.h >>> +++ b/arch/x86/include/asm/fpu/internal.h >>> @@ -578,7 +578,7 @@ static inline void switch_fpu_finish(str >>> * PKRU state is switched eagerly because it needs to be valid before we >>> * return to userland e.g. for a copy_to_user() operation. >>> */ >>> - if (current->mm) { >>> + if (!(current->flags & PF_KTHREAD)) { >>> pk = get_xsave_addr(&new_fpu->state.xsave, XFEATURE_PKRU); >>> if (pk) >>> pkru_val = pk->pkru; >>> >>> >> Why are we checking this at all? I actually tend to agree with the >> ->mm check more than PF_anything. If we have a user address space, >> then PKRU matters. If we don’t, then it doesn’t. > > Which PKRU matters? A kernel thread has always the default PKRU no > matter whether it uses a mm or not. It _cannot_ borrow the PKRU from the > mm owning process. There is no way, so let's not pretend there would be. >
Hmm. I guess PK_KTHREAD is consistent with switch_fpu_prepare() -- kernel threads have no FPU state.
It might be worth a loud comment here that kernel threads' PKRU is not context switched and that, if anyone wants kthread_use_mm() users to use anything other than the default PKRU, that they will need to change this.
So I guess your patch is okay.
--Andy
| |