On 3/20/18, 08:05, "Andy Lutomirski" <[email protected]> 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.
