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(a)linux.intel.com> Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> Cc: Colin Ian King <colin.king(a)canonical.com> Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> Cc: Arnaldo Carvalho de Melo <acme(a)redhat.com> Cc: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Kees Cook <keescook(a)chromium.org> Cc: Thomas Garnier <thgarnie(a)google.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Dmitry Vyukov <dvyukov(a)google.com> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: x86(a)kernel.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)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>. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --