[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen staging-4.9] x86/PV: check GDT/LDT limits during emulation
commit 754a531089a5becb85d43364af0675e0a8cc5475 Author: Jan Beulich <jbeulich@xxxxxxxx> AuthorDate: Mon Nov 4 15:02:25 2019 +0100 Commit: Jan Beulich <jbeulich@xxxxxxxx> CommitDate: Mon Nov 4 15:02:25 2019 +0100 x86/PV: check GDT/LDT limits during emulation Accesses beyond the LDT limit originating from emulation would trigger the ASSERT() in pv_map_ldt_shadow_page(). On production builds such accesses would cause an attempt to promote the touched page (offset from the present LDT base address) to a segment descriptor one. If this happens to succeed, guest user mode would be able to elevate its privileges to that of the guest kernel. This is particularly easy when there's no LDT at all, in which case the LDT base stored internally to Xen is simply zero. Also adjust the ASSERT() that was triggering: It was off by one to begin with, and for production builds we also better use ASSERT_UNREACHABLE() instead with suitable recovery code afterwards. This is XSA-298. Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> master commit: 93021cbe880a8013691a48d0febef8ed7d3e3ebd master date: 2019-10-31 16:08:16 +0100 --- xen/arch/x86/mm.c | 12 ++++++++++++ xen/arch/x86/traps.c | 19 ++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index f10d823f8e..bdc2682154 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -712,6 +712,18 @@ int map_ldt_shadow_page(unsigned int off) BUG_ON(unlikely(in_irq())); + /* + * Prior limit checking should guarantee this property. NB. This is + * safe as updates to the LDT can only be made by MMUEXT_SET_LDT to the + * current vcpu, and vcpu_reset() will block until this vcpu has been + * descheduled before continuing. + */ + if ( unlikely((off >> 3) >= v->arch.pv_vcpu.ldt_ents) ) + { + ASSERT_UNREACHABLE(); + return 0; + } + if ( is_pv_32bit_domain(d) ) gva = (u32)gva; guest_get_eff_kern_l1e(v, gva, &l1e); diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index f3c87053ed..53301574e2 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1639,7 +1639,14 @@ static int read_descriptor(unsigned int sel, { struct desc_struct desc; - if ( sel < 4) + if ( sel < 4 || + /* + * Don't apply the GDT limit here, as the selector may be a Xen + * provided one. __get_user() will fail (without taking further + * action) for ones falling in the gap between guest populated + * and Xen ones. + */ + ((sel & 4) && (sel >> 3) >= v->arch.pv_vcpu.ldt_ents) ) desc.b = desc.a = 0; else if ( __get_user(desc, (const struct desc_struct *)(!(sel & 4) @@ -1698,7 +1705,13 @@ static int read_gate_descriptor(unsigned int gate_sel, (!(gate_sel & 4) ? GDT_VIRT_START(v) : LDT_VIRT_START(v)) + (gate_sel >> 3); if ( (gate_sel < 4) || - ((gate_sel >= FIRST_RESERVED_GDT_BYTE) && !(gate_sel & 4)) || + /* + * We're interested in call gates only, which occupy a single + * seg_desc_t for 32-bit and a consecutive pair of them for 64-bit. + */ + ((gate_sel >> 3) + !is_pv_32bit_vcpu(v) >= + (gate_sel & 4 ? v->arch.pv_vcpu.ldt_ents + : v->arch.pv_vcpu.gdt_ents)) || __get_user(desc, pdesc) ) return 0; @@ -1717,7 +1730,7 @@ static int read_gate_descriptor(unsigned int gate_sel, if ( !is_pv_32bit_vcpu(v) ) { if ( (*ar & 0x1f00) != 0x0c00 || - (gate_sel >= FIRST_RESERVED_GDT_BYTE - 8 && !(gate_sel & 4)) || + /* Limit check done above already. */ __get_user(desc, pdesc + 1) || (desc.b & 0x1f00) ) return 0; -- generated by git-patchbot for /home/xen/git/xen.git#staging-4.9 _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |