On Fri, Nov 16, 2018 at 3:27 PM Chang S. Bae <[email protected]> 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 <[email protected]> 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.
