Messages in this thread | | | From | "Bae, Chang Seok" <> | Subject | Re: [PATCH 14/15] x86/fsgsbase/64: Support legacy behavior when FS/GS updated by ptracer | Date | Tue, 20 Mar 2018 16:33:35 +0000 |
| |
On 3/20/18, 08:05, "Andy Lutomirski" <luto@kernel.org> wrote: > I've also suggested something like this myself, but this approach is > far more complicated than the older approach. Was there something > that the old approach would break? If so, what? Sorry, I don't know your suggestion. Can you elaborate your suggestion?
>> + /* >> + * %fs setting goes to reload its base, when tracee >> + * resumes without FSGSBASE (legacy). Here with FSGSBASE >> + * FS base is (manually) fetched from GDT/LDT when needed. >> + */ >> + else if (static_cpu_has(X86_FEATURE_FSGSBASE) && >> + (value != 0) && (task->thread.fsindex != value)) >> + task->thread.fsbase = task_seg_base(task, value);
> The comment above should explain why you're checking this particular > condition. I find the fsindex != value check to be *very* surprising. > On a real CPU, writing some nonzero value to %fs does the same thing > regardless of what the old value of %fs was.
With FSGSBASE, when both index and base are not changed, base will be (always) fetched from GDT/LDT. This is not thought as legacy behavior we need to support, AFAIK.
> This is_fully_covered thing is IMO overcomplicated. Why not just make > a separate helper set_fsgs_index_and_base() and have putregs() call it > when both are set at once?
Using helper function here is exactly what I did at first. I thought this tag is simple enough and straightforward at the end. But I'm open to factor it out.
| |