Messages in this thread | | | From | Andy Lutomirski <> | Date | Sun, 18 Nov 2018 17:55:28 -0800 | Subject | Re: [PATCH v5] x86/fsgsbase/64: Fix the base write helper functions |
| |
On Fri, Nov 16, 2018 at 3:27 PM Chang S. Bae <chang.seok.bae@intel.com> wrote: > > The helper functions that purport to write the base should just write it > only. It shouldn't have magic optimizations to change the index. > > Make the index explicitly changed from the caller, instead of including > the code in the helpers. > > Subsequently, the task write helpers do not handle for the current task > anymore. The range check for a base value is also factored out, to > minimize code redundancy from the caller. > > v2: Fix further on the task write functions. Revert the changes on the > task read helpers. > > v3: Fix putreg(). Edit the changelog. > > v4: Update the task write helper functions and do_arch_prctl_64(). Fix > the comment in putreg(). > > v5: Fix preempt_disable() calls in do_arch_prctl_64()
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Ingo, Thomas: can we get this in x86/urgent, please?
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c > index ffae9b9740fd..4b8ee05dd6ad 100644 > --- a/arch/x86/kernel/ptrace.c > +++ b/arch/x86/kernel/ptrace.c > @@ -397,11 +397,12 @@ static int putreg(struct task_struct *child, > if (value >= TASK_SIZE_MAX) > return -EIO; > /* > - * When changing the FS base, use the same > - * mechanism as for do_arch_prctl_64(). > + * When changing the FS base, use do_arch_prctl_64() > + * to set the index to zero and to set the base > + * as requested. > */ > if (child->thread.fsbase != value) > - return x86_fsbase_write_task(child, value); > + return do_arch_prctl_64(child, ARCH_SET_FS, value);
FWIW, this logic is and was nonsensical, but it matches historical behavior, so I guess it's okay. I suspect that gdb only works by luck, since fs_base has a *higher* index than fs (and same for gs), which means that SETREGS with a nonzero fs or gs likely only works because the target almost always already has fs_base or gs_base == 0, so we bypass this entire mess.
Sigh. When you resubmit the full FSGSBASE series, I'll review the new code extra carefully.
| |