Messages in this thread | | | From | "Bae, Chang Seok" <> | Subject | Re: [PATCH] x86/fsgsbase/64: Fix the base write helper functions | Date | Wed, 31 Oct 2018 18:50:28 +0000 |
| |
> On Oct 30, 2018, at 14:25, Andy Lutomirski <luto@kernel.org> wrote: > > On Tue, Oct 30, 2018 at 10:28 AM Chang S. Bae <chang.seok.bae@intel.com> wrote: >> >> Factor out the code to change index from x86_fsbase_write_cpu() and >> x86_gsbase_write_cpu_inactive(). Now the code is located in >> do_arch_prctl_64(). >> > >> @@ -359,9 +351,7 @@ unsigned long x86_fsbase_read_task(struct task_struct *task) >> { >> unsigned long fsbase; >> >> - if (task == current) >> - fsbase = x86_fsbase_read_cpu(); >> - else if (task->thread.fsindex == 0) >> + if (task->thread.fsindex == 0) > > I'm okay with this change but, if you do it, please add: > > WARN_ON_ONCE(task == current); > > and make it be in a separate patch. >
Okay. Let me unchange those read functions, as it does what is claimed to do.
>> gsbase = task->thread.gsbase; >> else >> gsbase = x86_fsgsbase_read_task(task, task->thread.gsindex); >> @@ -392,12 +380,8 @@ int x86_fsbase_write_task(struct task_struct *task, unsigned long fsbase) >> if (unlikely(fsbase >= TASK_SIZE_MAX)) >> return -EPERM; >> >> - preempt_disable(); >> task->thread.fsbase = fsbase; >> - if (task == current) >> - x86_fsbase_write_cpu(fsbase); >> task->thread.fsindex = 0; > > I'm confused. You're still setting fsindex to zero here. >
These task write functions are getting thiner, although I agree the index update should move out.
> ret = x86_gsbase_write_task(task, arg); > if (ret == 0) { > /* ARCH_SET_GS has always overwritten the index and the base. Zero > is the most sensible value to put in the index, and is the only value > that makes any sense if FSGSBASE is unavailable. */ > if (task == current) > loadseg(GS, 0); > else > task->thread.gsindex = 0; > }
Thank you for the clarification. I think writing base should come right after loadseg().
Chang
| |