> On Oct 30, 2018, at 14:25, Andy Lutomirski <[email protected]> wrote:
>
> On Tue, Oct 30, 2018 at 10:28 AM Chang S. Bae <[email protected]>
> 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
