Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Wed, Dec 06, 2017 at 11:23:59PM -0800, Ricardo Neri wrote:
> In UMIP emulation we can potentially access the LDT twice. Once when
> determining the base address of the code segment and again when determining
> the base address and limit of the segment in which the result of the
> emulation is written. I guess that mm->context.ldt_seq needs to not change
> not only while decoding a particular linear address but across these two
> linear address decodings.

Yap, stuff which needs to see an *unchanged* LDT should use the cookie
to verify that and the LDT code should change the cookie when the LDT
is modified.

> Sure, I will look into implementing this idea and post patches for it.

Thanks!

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Wed, Dec 06, 2017 at 11:23:59PM -0800, Ricardo Neri wrote:
> In UMIP emulation we can potentially access the LDT twice. Once when
> determining the base address of the code segment and again when determining
> the base address and limit of the segment in which the result of the
> emulation is written. I guess that mm->context.ldt_seq needs to not change
> not only while decoding a particular linear address but across these two
> linear address decodings.

Yap, stuff which needs to see an *unchanged* LDT should use the cookie
to verify that and the LDT code should change the cookie when the LDT
is modified.

> Sure, I will look into implementing this idea and post patches for it.

Thanks!

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Wed, Dec 06, 2017 at 11:26:05PM -0800, Ricardo Neri wrote:
> At the moment MPX and UMIP are using the insn-eval decoder to determine
> linear addresses.

If we're keeping a whole instruction decoder in the kernel, it better
be designed generically enough and usable (and used) by everything that
needs it.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Wed, Dec 06, 2017 at 11:26:05PM -0800, Ricardo Neri wrote:
> At the moment MPX and UMIP are using the insn-eval decoder to determine
> linear addresses.

If we're keeping a whole instruction decoder in the kernel, it better
be designed generically enough and usable (and used) by everything that
needs it.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Ricardo Neri]

On Tue, Dec 05, 2017 at 10:29:33PM +0100, Borislav Petkov wrote:
> On Tue, Dec 05, 2017 at 07:38:45PM +0100, Peter Zijlstra wrote:
> > Sorry what? So either this code is broken because it has IRQs enabled,
> > or its broken because its trying to acquire a mutex with IRQs disabled.
> > Which is it?
> 
> Well, lemme try to sum up what Peter, Thomas and I discussed on IRC:
> 
> The problem is that there's no guarantee userspace won't change the LDT
> from under us while the UMIP code runs in the insn decoder.

Yes, I see the problem now.
> 
> So, we need a way to be able to query the desc fields the insn decoder
> needs *and* when the LDT changes through the syscall, to detect that
> case and handle it gracefully in the decoder.
> 
> So Thomas' idea is to keep a mm->context.ldt_seq sequence number which
> gets incremented (and wraps around) everytime a LDT changes.
> 
> That sequence number, i.e., cookie, gets handed down into the decoder
> and it uses it during desc lookup. If the sequence number changes, the
> decoder and the UMIP code must abort the emulation.

In UMIP emulation we can potentially access the LDT twice. Once when
determining the base address of the code segment and again when determining
the base address and limit of the segment in which the result of the
emulation is written. I guess that mm->context.ldt_seq needs to not change
not only while decoding a particular linear address but across these two
linear address decodings.
> 
> The lookup code needs to do that with IRQs disabled, of course, to
> protect itself from IPIs which could change the LDT.
> 
> I *think* this is the gist of what we talked about, tglx, please correct
> me if I missed something.
> 
> So, Ricardo, please take a look at fixing that as otherwise the UMIP
> code would choke and possibly rely on wrong data. If there are any
> questions, don't hesitate to ask.

Sure, I will look into implementing this idea and post patches for it.

Thanks and BR,
Ricardo

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Ricardo Neri]

On Tue, Dec 05, 2017 at 10:29:33PM +0100, Borislav Petkov wrote:
> On Tue, Dec 05, 2017 at 07:38:45PM +0100, Peter Zijlstra wrote:
> > Sorry what? So either this code is broken because it has IRQs enabled,
> > or its broken because its trying to acquire a mutex with IRQs disabled.
> > Which is it?
> 
> Well, lemme try to sum up what Peter, Thomas and I discussed on IRC:
> 
> The problem is that there's no guarantee userspace won't change the LDT
> from under us while the UMIP code runs in the insn decoder.

Yes, I see the problem now.
> 
> So, we need a way to be able to query the desc fields the insn decoder
> needs *and* when the LDT changes through the syscall, to detect that
> case and handle it gracefully in the decoder.
> 
> So Thomas' idea is to keep a mm->context.ldt_seq sequence number which
> gets incremented (and wraps around) everytime a LDT changes.
> 
> That sequence number, i.e., cookie, gets handed down into the decoder
> and it uses it during desc lookup. If the sequence number changes, the
> decoder and the UMIP code must abort the emulation.

In UMIP emulation we can potentially access the LDT twice. Once when
determining the base address of the code segment and again when determining
the base address and limit of the segment in which the result of the
emulation is written. I guess that mm->context.ldt_seq needs to not change
not only while decoding a particular linear address but across these two
linear address decodings.
> 
> The lookup code needs to do that with IRQs disabled, of course, to
> protect itself from IPIs which could change the LDT.
> 
> I *think* this is the gist of what we talked about, tglx, please correct
> me if I missed something.
> 
> So, Ricardo, please take a look at fixing that as otherwise the UMIP
> code would choke and possibly rely on wrong data. If there are any
> questions, don't hesitate to ask.

Sure, I will look into implementing this idea and post patches for it.

Thanks and BR,
Ricardo

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Ricardo Neri]

On Tue, Dec 05, 2017 at 07:14:56PM +0100, Borislav Petkov wrote:
> 
> But, if other code is going to use those functions - and I believe
> that's the idea - otherwise they wouldn't be in arch/x86/lib/ 

At the moment MPX and UMIP are using the insn-eval decoder to determine
linear addresses.

Thanks and BR,
Ricardo

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Ricardo Neri]

On Tue, Dec 05, 2017 at 07:14:56PM +0100, Borislav Petkov wrote:
> 
> But, if other code is going to use those functions - and I believe
> that's the idea - otherwise they wouldn't be in arch/x86/lib/ 

At the moment MPX and UMIP are using the insn-eval decoder to determine
linear addresses.

Thanks and BR,
Ricardo

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Tue, Dec 05, 2017 at 07:38:45PM +0100, Peter Zijlstra wrote:
> Sorry what? So either this code is broken because it has IRQs enabled,
> or its broken because its trying to acquire a mutex with IRQs disabled.
> Which is it?

Well, lemme try to sum up what Peter, Thomas and I discussed on IRC:

The problem is that there's no guarantee userspace won't change the LDT
from under us while the UMIP code runs in the insn decoder.

So, we need a way to be able to query the desc fields the insn decoder
needs *and* when the LDT changes through the syscall, to detect that
case and handle it gracefully in the decoder.

So Thomas' idea is to keep a mm->context.ldt_seq sequence number which
gets incremented (and wraps around) everytime a LDT changes.

That sequence number, i.e., cookie, gets handed down into the decoder
and it uses it during desc lookup. If the sequence number changes, the
decoder and the UMIP code must abort the emulation.

The lookup code needs to do that with IRQs disabled, of course, to
protect itself from IPIs which could change the LDT.

I *think* this is the gist of what we talked about, tglx, please correct
me if I missed something.

So, Ricardo, please take a look at fixing that as otherwise the UMIP
code would choke and possibly rely on wrong data. If there are any
questions, don't hesitate to ask.

Thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Tue, Dec 05, 2017 at 07:38:45PM +0100, Peter Zijlstra wrote:
> Sorry what? So either this code is broken because it has IRQs enabled,
> or its broken because its trying to acquire a mutex with IRQs disabled.
> Which is it?

Well, lemme try to sum up what Peter, Thomas and I discussed on IRC:

The problem is that there's no guarantee userspace won't change the LDT
from under us while the UMIP code runs in the insn decoder.

So, we need a way to be able to query the desc fields the insn decoder
needs *and* when the LDT changes through the syscall, to detect that
case and handle it gracefully in the decoder.

So Thomas' idea is to keep a mm->context.ldt_seq sequence number which
gets incremented (and wraps around) everytime a LDT changes.

That sequence number, i.e., cookie, gets handed down into the decoder
and it uses it during desc lookup. If the sequence number changes, the
decoder and the UMIP code must abort the emulation.

The lookup code needs to do that with IRQs disabled, of course, to
protect itself from IPIs which could change the LDT.

I *think* this is the gist of what we talked about, tglx, please correct
me if I missed something.

So, Ricardo, please take a look at fixing that as otherwise the UMIP
code would choke and possibly rely on wrong data. If there are any
questions, don't hesitate to ask.

Thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Peter Zijlstra]

On Tue, Dec 05, 2017 at 07:14:56PM +0100, Borislav Petkov wrote:
> On Tue, Dec 05, 2017 at 06:48:44PM +0100, Peter Zijlstra wrote:
> > This is broken right? You unlock and then return @desc, which afaict can
> > at that point get freed by free_ldt_struct().
> > 
> > Something like the below ought to cure; although its not entirely
> > pretty either.
> 
> Right.
> 
> Or, instead of introducing all the locking, we could also not do
> anything because all that code runs inside fixup_umip_exception() so the
> desc will be valid there.

Sorry what? So either this code is broken because it has IRQs enabled,
or its broken because its trying to acquire a mutex with IRQs disabled.
Which is it?

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Peter Zijlstra]

On Tue, Dec 05, 2017 at 07:14:56PM +0100, Borislav Petkov wrote:
> On Tue, Dec 05, 2017 at 06:48:44PM +0100, Peter Zijlstra wrote:
> > This is broken right? You unlock and then return @desc, which afaict can
> > at that point get freed by free_ldt_struct().
> > 
> > Something like the below ought to cure; although its not entirely
> > pretty either.
> 
> Right.
> 
> Or, instead of introducing all the locking, we could also not do
> anything because all that code runs inside fixup_umip_exception() so the
> desc will be valid there.

Sorry what? So either this code is broken because it has IRQs enabled,
or its broken because its trying to acquire a mutex with IRQs disabled.
Which is it?

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Tue, Dec 05, 2017 at 06:48:44PM +0100, Peter Zijlstra wrote:
> This is broken right? You unlock and then return @desc, which afaict can
> at that point get freed by free_ldt_struct().
> 
> Something like the below ought to cure; although its not entirely
> pretty either.

Right.

Or, instead of introducing all the locking, we could also not do
anything because all that code runs inside fixup_umip_exception() so the
desc will be valid there.

But, if other code is going to use those functions - and I believe
that's the idea - otherwise they wouldn't be in arch/x86/lib/ - we
should convert all those functions to return directly the desc field
which is requested by the respective caller.

I.e., get_desc() will be called by a wrapper which returns desc base or
desc limit or whatever...

In the case where desc has been freed, it should return error, of
course.

How doed that sound?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Borislav Petkov]

On Tue, Dec 05, 2017 at 06:48:44PM +0100, Peter Zijlstra wrote:
> This is broken right? You unlock and then return @desc, which afaict can
> at that point get freed by free_ldt_struct().
> 
> Something like the below ought to cure; although its not entirely
> pretty either.

Right.

Or, instead of introducing all the locking, we could also not do
anything because all that code runs inside fixup_umip_exception() so the
desc will be valid there.

But, if other code is going to use those functions - and I believe
that's the idea - otherwise they wouldn't be in arch/x86/lib/ - we
should convert all those functions to return directly the desc field
which is requested by the respective caller.

I.e., get_desc() will be called by a wrapper which returns desc base or
desc limit or whatever...

In the case where desc has been freed, it should return error, of
course.

How doed that sound?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Peter Zijlstra]

On Wed, Nov 01, 2017 at 02:00:28PM -0700, tip-bot for Ricardo Neri wrote:
> +static struct desc_struct *get_desc(unsigned short sel)
> +{
> + struct desc_ptr gdt_desc = {0, 0};
> + unsigned long desc_base;
> +
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> + if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
> + struct desc_struct *desc = NULL;
> + struct ldt_struct *ldt;
> +
> + /* Bits [15:3] contain the index of the desired entry. */
> + sel >>= 3;
> +
> + mutex_lock(>active_mm->context.lock);
> + ldt = current->active_mm->context.ldt;
> + if (ldt && sel < ldt->nr_entries)
> + desc = >entries[sel];
> +
> + mutex_unlock(>active_mm->context.lock);
> +
> + return desc;
> + }
> +#endif

This is broken right? You unlock and then return @desc, which afaict can
at that point get freed by free_ldt_struct().

Something like the below ought to cure; although its not entirely
pretty either.

---

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index e664058c4491..c234ef2b4430 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -572,6 +572,11 @@ static struct desc_struct *get_desc(unsigned short sel)
struct desc_ptr gdt_desc = {0, 0};
unsigned long desc_base;
 
+   /*
+* Relies on IRQs being disabled to serialize against the LDT.
+*/
+   lockdep_assert_irqs_disabled();
+
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
struct desc_struct *desc = NULL;
@@ -580,13 +585,10 @@ static struct desc_struct *get_desc(unsigned short sel)
/* Bits [15:3] contain the index of the desired entry. */
sel >>= 3;
 
-   mutex_lock(>active_mm->context.lock);
ldt = current->active_mm->context.ldt;
if (ldt && sel < ldt->nr_entries)
desc = >entries_va[sel];
 
-   mutex_unlock(>active_mm->context.lock);
-
return desc;
}
 #endif
@@ -626,6 +628,7 @@ static struct desc_struct *get_desc(unsigned short sel)
  */
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
 {
+   unsigned long base, flags;
struct desc_struct *desc;
short sel;
 
@@ -664,11 +667,15 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int 
seg_reg_idx)
if (!sel)
return -1L;
 
+   base = -1;
+
+   local_irq_save(flags);
desc = get_desc(sel);
-   if (!desc)
-   return -1L;
+   if (desc)
+   base = get_desc_base(desc);
+   local_irq_restore(flags);
 
-   return get_desc_base(desc);
+   return base;
 }
 
 /**
@@ -690,8 +697,8 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int 
seg_reg_idx)
  */
 static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 {
+   unsigned long flags, limit = 0;
struct desc_struct *desc;
-   unsigned long limit;
short sel;
 
sel = get_segment_selector(regs, seg_reg_idx);
@@ -704,19 +711,20 @@ static unsigned long get_seg_limit(struct pt_regs *regs, 
int seg_reg_idx)
if (!sel)
return 0;
 
+   local_irq_save(flags);
desc = get_desc(sel);
-   if (!desc)
-   return 0;
-
-   /*
-* If the granularity bit is set, the limit is given in multiples
-* of 4096. This also means that the 12 least significant bits are
-* not tested when checking the segment limits. In practice,
-* this means that the segment ends in (limit << 12) + 0xfff.
-*/
-   limit = get_desc_limit(desc);
-   if (desc->g)
-   limit = (limit << 12) + 0xfff;
+   if (desc) {
+   /*
+* If the granularity bit is set, the limit is given in 
multiples
+* of 4096. This also means that the 12 least significant bits 
are
+* not tested when checking the segment limits. In practice,
+* this means that the segment ends in (limit << 12) + 0xfff.
+*/
+   limit = get_desc_limit(desc);
+   if (desc->g)
+   limit = (limit << 12) + 0xfff;
+   }
+   local_irq_restore(flags);
 
return limit;
 }
@@ -740,19 +748,23 @@ static unsigned long get_seg_limit(struct pt_regs *regs, 
int seg_reg_idx)
 int insn_get_code_seg_params(struct pt_regs *regs)
 {
struct desc_struct *desc;
+   unsigned long flags;
+   int ret = -EINVAL;
short sel;
 
-   if (v8086_mode(regs))
+   if (v8086_mode(regs)) {
/* Address and operand size are both 16-bit. */
return INSN_CODE_SEG_PARAMS(2, 2);
+   }
 
sel = get_segment_selector(regs, INAT_SEG_REG_CS);
if (sel < 0)
return sel;
 
+   

Re: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[Peter Zijlstra]

On Wed, Nov 01, 2017 at 02:00:28PM -0700, tip-bot for Ricardo Neri wrote:
> +static struct desc_struct *get_desc(unsigned short sel)
> +{
> + struct desc_ptr gdt_desc = {0, 0};
> + unsigned long desc_base;
> +
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> + if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
> + struct desc_struct *desc = NULL;
> + struct ldt_struct *ldt;
> +
> + /* Bits [15:3] contain the index of the desired entry. */
> + sel >>= 3;
> +
> + mutex_lock(>active_mm->context.lock);
> + ldt = current->active_mm->context.ldt;
> + if (ldt && sel < ldt->nr_entries)
> + desc = >entries[sel];
> +
> + mutex_unlock(>active_mm->context.lock);
> +
> + return desc;
> + }
> +#endif

This is broken right? You unlock and then return @desc, which afaict can
at that point get freed by free_ldt_struct().

Something like the below ought to cure; although its not entirely
pretty either.

---

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index e664058c4491..c234ef2b4430 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -572,6 +572,11 @@ static struct desc_struct *get_desc(unsigned short sel)
struct desc_ptr gdt_desc = {0, 0};
unsigned long desc_base;
 
+   /*
+* Relies on IRQs being disabled to serialize against the LDT.
+*/
+   lockdep_assert_irqs_disabled();
+
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
struct desc_struct *desc = NULL;
@@ -580,13 +585,10 @@ static struct desc_struct *get_desc(unsigned short sel)
/* Bits [15:3] contain the index of the desired entry. */
sel >>= 3;
 
-   mutex_lock(>active_mm->context.lock);
ldt = current->active_mm->context.ldt;
if (ldt && sel < ldt->nr_entries)
desc = >entries_va[sel];
 
-   mutex_unlock(>active_mm->context.lock);
-
return desc;
}
 #endif
@@ -626,6 +628,7 @@ static struct desc_struct *get_desc(unsigned short sel)
  */
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
 {
+   unsigned long base, flags;
struct desc_struct *desc;
short sel;
 
@@ -664,11 +667,15 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int 
seg_reg_idx)
if (!sel)
return -1L;
 
+   base = -1;
+
+   local_irq_save(flags);
desc = get_desc(sel);
-   if (!desc)
-   return -1L;
+   if (desc)
+   base = get_desc_base(desc);
+   local_irq_restore(flags);
 
-   return get_desc_base(desc);
+   return base;
 }
 
 /**
@@ -690,8 +697,8 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int 
seg_reg_idx)
  */
 static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 {
+   unsigned long flags, limit = 0;
struct desc_struct *desc;
-   unsigned long limit;
short sel;
 
sel = get_segment_selector(regs, seg_reg_idx);
@@ -704,19 +711,20 @@ static unsigned long get_seg_limit(struct pt_regs *regs, 
int seg_reg_idx)
if (!sel)
return 0;
 
+   local_irq_save(flags);
desc = get_desc(sel);
-   if (!desc)
-   return 0;
-
-   /*
-* If the granularity bit is set, the limit is given in multiples
-* of 4096. This also means that the 12 least significant bits are
-* not tested when checking the segment limits. In practice,
-* this means that the segment ends in (limit << 12) + 0xfff.
-*/
-   limit = get_desc_limit(desc);
-   if (desc->g)
-   limit = (limit << 12) + 0xfff;
+   if (desc) {
+   /*
+* If the granularity bit is set, the limit is given in 
multiples
+* of 4096. This also means that the 12 least significant bits 
are
+* not tested when checking the segment limits. In practice,
+* this means that the segment ends in (limit << 12) + 0xfff.
+*/
+   limit = get_desc_limit(desc);
+   if (desc->g)
+   limit = (limit << 12) + 0xfff;
+   }
+   local_irq_restore(flags);
 
return limit;
 }
@@ -740,19 +748,23 @@ static unsigned long get_seg_limit(struct pt_regs *regs, 
int seg_reg_idx)
 int insn_get_code_seg_params(struct pt_regs *regs)
 {
struct desc_struct *desc;
+   unsigned long flags;
+   int ret = -EINVAL;
short sel;
 
-   if (v8086_mode(regs))
+   if (v8086_mode(regs)) {
/* Address and operand size are both 16-bit. */
return INSN_CODE_SEG_PARAMS(2, 2);
+   }
 
sel = get_segment_selector(regs, INAT_SEG_REG_CS);
if (sel < 0)
return sel;
 
+   

[tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[tip-bot for Ricardo Neri]

Commit-ID:  670f928ba09b06712da34a3c44be6c8fa561fb19
Gitweb: https://git.kernel.org/tip/670f928ba09b06712da34a3c44be6c8fa561fb19
Author: Ricardo Neri 
AuthorDate: Fri, 27 Oct 2017 13:25:41 -0700
Committer:  Thomas Gleixner 
CommitDate: Wed, 1 Nov 2017 21:50:12 +0100

x86/insn-eval: Add utility function to get segment descriptor

The segment descriptor contains information that is relevant to how linear
addresses 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 to 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 a prerequisite for functions in subsequent commits that
will obtain the aforementioned attributes of the segment descriptor.

Improvements-by: Borislav Petkov 
Signed-off-by: Ricardo Neri 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Borislav Petkov 
Cc: "Michael S. Tsirkin" 
Cc: Peter Zijlstra 
Cc: Dave Hansen 
Cc: ricardo.n...@intel.com
Cc: Adrian Hunter 
Cc: Paul Gortmaker 
Cc: Huang Rui 
Cc: Qiaowei Ren 
Cc: Shuah Khan 
Cc: Kees Cook 
Cc: Jonathan Corbet 
Cc: Jiri Slaby 
Cc: Dmitry Vyukov 
Cc: "Ravi V. Shankar" 
Cc: Chris Metcalf 
Cc: Brian Gerst 
Cc: Arnaldo Carvalho de Melo 
Cc: Andy Lutomirski 
Cc: Colin Ian King 
Cc: Chen Yucong 
Cc: Adam Buchbinder 
Cc: Vlastimil Babka 
Cc: Lorenzo Stoakes 
Cc: Masami Hiramatsu 
Cc: Paolo Bonzini 
Cc: Andrew Morton 
Cc: Thomas Garnier 
Link: 
https://lkml.kernel.org/r/1509135945-13762-15-git-send-email-ricardo.neri-calde...@linux.intel.com

---
 arch/x86/lib/insn-eval.c | 57 
 1 file changed, 57 insertions(+)

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 6a902b1..d85e840 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -6,9 +6,13 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #undef pr_fmt
@@ -469,6 +473,59 @@ static int get_reg_offset(struct insn *insn, struct 
pt_regs *regs,
 }
 
 /**
+ * get_desc() - Obtain pointer to a segment descriptor
+ * @sel:   Segment selector
+ *
+ * Given a segment selector, obtain a pointer to the segment descriptor.
+ * Both global and local descriptor tables are supported.
+ *
+ * Returns:
+ *
+ * Pointer to segment descriptor on success.
+ *
+ * NULL on error.
+ */
+static struct desc_struct *get_desc(unsigned short sel)
+{
+   struct desc_ptr gdt_desc = {0, 0};
+   unsigned long desc_base;
+
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
+   if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
+   struct desc_struct *desc = NULL;
+   struct ldt_struct *ldt;
+
+   /* Bits [15:3] contain the index of the desired entry. */
+   sel >>= 3;
+
+   mutex_lock(>active_mm->context.lock);
+   ldt = current->active_mm->context.ldt;
+   if (ldt && sel < ldt->nr_entries)
+   desc = >entries[sel];
+
+   mutex_unlock(>active_mm->context.lock);
+
+   return desc;
+   }
+#endif
+   native_store_gdt(_desc);
+
+   /*
+* Segment descriptors have a size of 8 bytes. Thus, the index is
+* multiplied by 8 to obtain the memory offset of the desired descriptor
+* from the base of the GDT. As bits [15:3] of the segment selector
+* contain the index, it can be regarded as multiplied by 8 already.
+* All that remains is to clear bits [2:0].
+*/
+   desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
+
+   if (desc_base > gdt_desc.size)
+   return NULL;
+
+   return (struct desc_struct *)(gdt_desc.address + desc_base);
+}
+
+/**
  * insn_get_modrm_rm_off() - Obtain register in r/m part of the ModRM byte
  * @insn:  Instruction containing the ModRM byte
  * @regs:  Register values as seen when entering kernel mode

[tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor

[tip-bot for Ricardo Neri]

Commit-ID:  670f928ba09b06712da34a3c44be6c8fa561fb19
Gitweb: https://git.kernel.org/tip/670f928ba09b06712da34a3c44be6c8fa561fb19
Author: Ricardo Neri 
AuthorDate: Fri, 27 Oct 2017 13:25:41 -0700
Committer:  Thomas Gleixner 
CommitDate: Wed, 1 Nov 2017 21:50:12 +0100

x86/insn-eval: Add utility function to get segment descriptor

The segment descriptor contains information that is relevant to how linear
addresses 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 to 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 a prerequisite for functions in subsequent commits that
will obtain the aforementioned attributes of the segment descriptor.

Improvements-by: Borislav Petkov 
Signed-off-by: Ricardo Neri 
Signed-off-by: Thomas Gleixner 
Reviewed-by: Borislav Petkov 
Cc: "Michael S. Tsirkin" 
Cc: Peter Zijlstra 
Cc: Dave Hansen 
Cc: ricardo.n...@intel.com
Cc: Adrian Hunter 
Cc: Paul Gortmaker 
Cc: Huang Rui 
Cc: Qiaowei Ren 
Cc: Shuah Khan 
Cc: Kees Cook 
Cc: Jonathan Corbet 
Cc: Jiri Slaby 
Cc: Dmitry Vyukov 
Cc: "Ravi V. Shankar" 
Cc: Chris Metcalf 
Cc: Brian Gerst 
Cc: Arnaldo Carvalho de Melo 
Cc: Andy Lutomirski 
Cc: Colin Ian King 
Cc: Chen Yucong 
Cc: Adam Buchbinder 
Cc: Vlastimil Babka 
Cc: Lorenzo Stoakes 
Cc: Masami Hiramatsu 
Cc: Paolo Bonzini 
Cc: Andrew Morton 
Cc: Thomas Garnier 
Link: 
https://lkml.kernel.org/r/1509135945-13762-15-git-send-email-ricardo.neri-calde...@linux.intel.com

---
 arch/x86/lib/insn-eval.c | 57 
 1 file changed, 57 insertions(+)

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 6a902b1..d85e840 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -6,9 +6,13 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #undef pr_fmt
@@ -469,6 +473,59 @@ static int get_reg_offset(struct insn *insn, struct 
pt_regs *regs,
 }
 
 /**
+ * get_desc() - Obtain pointer to a segment descriptor
+ * @sel:   Segment selector
+ *
+ * Given a segment selector, obtain a pointer to the segment descriptor.
+ * Both global and local descriptor tables are supported.
+ *
+ * Returns:
+ *
+ * Pointer to segment descriptor on success.
+ *
+ * NULL on error.
+ */
+static struct desc_struct *get_desc(unsigned short sel)
+{
+   struct desc_ptr gdt_desc = {0, 0};
+   unsigned long desc_base;
+
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
+   if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
+   struct desc_struct *desc = NULL;
+   struct ldt_struct *ldt;
+
+   /* Bits [15:3] contain the index of the desired entry. */
+   sel >>= 3;
+
+   mutex_lock(>active_mm->context.lock);
+   ldt = current->active_mm->context.ldt;
+   if (ldt && sel < ldt->nr_entries)
+   desc = >entries[sel];
+
+   mutex_unlock(>active_mm->context.lock);
+
+   return desc;
+   }
+#endif
+   native_store_gdt(_desc);
+
+   /*
+* Segment descriptors have a size of 8 bytes. Thus, the index is
+* multiplied by 8 to obtain the memory offset of the desired descriptor
+* from the base of the GDT. As bits [15:3] of the segment selector
+* contain the index, it can be regarded as multiplied by 8 already.
+* All that remains is to clear bits [2:0].
+*/
+   desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
+
+   if (desc_base > gdt_desc.size)
+   return NULL;
+
+   return (struct desc_struct *)(gdt_desc.address + desc_base);
+}
+
+/**
  * insn_get_modrm_rm_off() - Obtain register in r/m part of the ModRM byte
  * @insn:  Instruction containing the ModRM byte
  * @regs:  Register values as seen when entering kernel mode