On Tue, Mar 07, 2017 at 04:32:40PM -0800, Ricardo Neri wrote:
The segment descriptor contains information that is relevant to how linear address need to be computed. It contains the default size of addresses as well as the base address of the segment. Thus, given a segment selector, we ought look at segment descriptor to correctly calculate the linear address.
In protected mode, the segment selector might indicate a segment descriptor from either the global descriptor table or a local descriptor table. Both cases are considered in this function.
This function is the initial implementation for subsequent functions that will obtain the aforementioned attributes of the segment descriptor.
Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Adam Buchbinder adam.buchbinder@gmail.com Cc: Colin Ian King colin.king@canonical.com Cc: Lorenzo Stoakes lstoakes@gmail.com Cc: Qiaowei Ren qiaowei.ren@intel.com Cc: Arnaldo Carvalho de Melo acme@redhat.com Cc: Masami Hiramatsu mhiramat@kernel.org Cc: Adrian Hunter adrian.hunter@intel.com Cc: Kees Cook keescook@chromium.org Cc: Thomas Garnier thgarnie@google.com Cc: Peter Zijlstra peterz@infradead.org Cc: Borislav Petkov bp@suse.de Cc: Dmitry Vyukov dvyukov@google.com Cc: Ravi V. Shankar ravi.v.shankar@intel.com Cc: x86@kernel.org Signed-off-by: Ricardo Neri ricardo.neri-calderon@linux.intel.com
arch/x86/lib/insn-eval.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index 8d45df8..8608adf 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -5,9 +5,13 @@ */ #include <linux/kernel.h> #include <linux/string.h> +#include <asm/desc_defs.h> +#include <asm/desc.h> #include <asm/inat.h> #include <asm/insn.h> #include <asm/insn-eval.h> +#include <asm/ldt.h> +#include <linux/mmu_context.h> #include <asm/vm86.h>
enum reg_type { @@ -294,6 +298,63 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, }
/**
- get_desc() - Obtain address of segment descriptor
- @seg: Segment selector
Maybe that should be
@sel
if it is a sel-ector. :)
And using "sel" makes more sense then when you look at:
desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
for example:
- @desc: Pointer to the selected segment descriptor
- Given a segment selector, obtain a memory pointer to the segment
s/memory //
- descriptor. Both global and local descriptor tables are supported.
- desc will contain the address of the descriptor.
- Return: 0 if success, -EINVAL if failure
Why isn't this function returning the pointer or NULL on error? Maybe the later patches have an answer and I'll discover it if I continue reviewing :)
- */
+static int get_desc(unsigned short seg, struct desc_struct **desc) +{
- struct desc_ptr gdt_desc = {0, 0};
- unsigned long desc_base;
- if (!desc)
return -EINVAL;
- desc_base = seg & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
That looks useless as you're doing it below again.
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
- if ((seg & SEGMENT_TI_MASK) == SEGMENT_LDT) {
seg >>= 3;
mutex_lock(¤t->active_mm->context.lock);
if (unlikely(!current->active_mm->context.ldt ||
Is that really a fast path to complicate the if-test with an unlikely()? If not, you don't really need it.
seg >= current->active_mm->context.ldt->size)) {
ldt->size is the size of the descriptor table but you've shifted seg by 3. That selector index is shifted by 3 (to the left) to form an offset into the descriptor table because the entries there are 8 bytes.
So I *think* you wanna use the "useless" desc_base above... :)
*desc = NULL;
mutex_unlock(¤t->active_mm->context.lock);
return -EINVAL;
}
*desc = ¤t->active_mm->context.ldt->entries[seg];
... and seg here as it is an index into the table.
mutex_unlock(¤t->active_mm->context.lock);
return 0;
- }
+#endif
- native_store_gdt(&gdt_desc);
- /*
* Bits [15:3] of the segment selector contain the index. Such
* index needs to be multiplied by 8.
... because <insert reason I typed in above>.