| Message ID | 1488204198-23948-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 27.02.17 at 15:03, <andrew.cooper3@citrix.com> wrote: > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -76,6 +76,7 @@ > /* Internally used only flags. */ > #define PFEC_page_paged (1U<<16) > #define PFEC_page_shared (1U<<17) > +#define PFEC_implicit (1U<<18) /* Pagewalk input for ldt/gdt/idt/tr accesses. */ PFEC_implicit makes it kind of implicit what implicit here means, but since anything more explicit would likely also be quite a bit longer, let's go with this for now: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
At 14:03 +0000 on 27 Feb (1488204192), Andrew Cooper wrote: > All actions which refer to the active ldt/gdt/idt or task register > (e.g. loading a new segment selector) are known as implicit supervisor > accesses, even when the access originates from user code. > > The distinction is necessary in the pagewalk when SMAP is enabled. Refer to > Intel SDM Vol 3 "Access Rights" for the exact details. > > Introduce a new pagewalk input, and make use of the new system segment > references in hvmemul_{read,write}(). While modifying those areas, move the > calculation of the appropriate pagewalk input before its first use. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Tim Deegan <tim@xen.org>
On 27/02/17 14:03, Andrew Cooper wrote: > All actions which refer to the active ldt/gdt/idt or task register > (e.g. loading a new segment selector) are known as implicit supervisor > accesses, even when the access originates from user code. > > The distinction is necessary in the pagewalk when SMAP is enabled. Refer to > Intel SDM Vol 3 "Access Rights" for the exact details. > > Introduce a new pagewalk input, and make use of the new system segment > references in hvmemul_{read,write}(). While modifying those areas, move the > calculation of the appropriate pagewalk input before its first use. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: George Dunlap <george.dunlap@eu.citrix.com> > CC: Tim Deegan <tim@xen.org> > --- > xen/arch/x86/hvm/emulate.c | 18 ++++++++++-------- > xen/arch/x86/mm/guest_walk.c | 4 ++++ > xen/include/asm-x86/processor.h | 1 + > 3 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index f24d289..9b32bca 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -783,6 +783,11 @@ static int __hvmemul_read( > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > int rc; > > + if ( is_x86_system_segment(seg) ) > + pfec |= PFEC_implicit; > + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) > + pfec |= PFEC_user_mode; > + > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr); > if ( rc != X86EMUL_OKAY || !bytes ) > @@ -793,10 +798,6 @@ static int __hvmemul_read( > (vio->mmio_gla == (addr & PAGE_MASK)) ) > return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec, hvmemul_ctxt, 1); > > - if ( (seg != x86_seg_none) && > - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) > - pfec |= PFEC_user_mode; > - > rc = ((access_type == hvm_access_insn_fetch) ? > hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) : > hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo)); > @@ -893,6 +894,11 @@ static int hvmemul_write( > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > int rc; > > + if ( is_x86_system_segment(seg) ) > + pfec |= PFEC_implicit; > + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) > + pfec |= PFEC_user_mode; > + > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); > if ( rc != X86EMUL_OKAY || !bytes ) > @@ -902,10 +908,6 @@ static int hvmemul_write( > (vio->mmio_gla == (addr & PAGE_MASK)) ) > return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec, hvmemul_ctxt, 1); > > - if ( (seg != x86_seg_none) && > - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) > - pfec |= PFEC_user_mode; > - > rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo); > > switch ( rc ) > diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c > index faaf70c..4f8d0e3 100644 > --- a/xen/arch/x86/mm/guest_walk.c > +++ b/xen/arch/x86/mm/guest_walk.c > @@ -161,6 +161,10 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, > bool_t pse1G = 0, pse2M = 0; > p2m_query_t qt = P2M_ALLOC | P2M_UNSHARE; > > + /* Only implicit supervisor data accesses exist. */ > + ASSERT(!(pfec & PFEC_implicit) || > + !(pfec & (PFEC_insn_fetch|PFEC_user_mode))); > + > perfc_incr(guest_walk); > memset(gw, 0, sizeof(*gw)); > gw->va = va; > diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h > index dda8b83..d3ba8ea 100644 > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -76,6 +76,7 @@ > /* Internally used only flags. */ > #define PFEC_page_paged (1U<<16) > #define PFEC_page_shared (1U<<17) > +#define PFEC_implicit (1U<<18) /* Pagewalk input for ldt/gdt/idt/tr accesses. */ > > /* Other exception error code values. */ > #define X86_XEC_EXT (_AC(1,U) << 0) >
On 27/02/17 14:03, Andrew Cooper wrote: > All actions which refer to the active ldt/gdt/idt or task register > (e.g. loading a new segment selector) are known as implicit supervisor > accesses, even when the access originates from user code. It turns out that this has a bugfix in it which I hadn't realised. I have added: "Right away, this fixes a bug during userspace emulation where a pagewalk for a system table was (incorrectly) performed as a user access, causing an access violation in the common case, as system tables reside on supervisor mappings." ~Andrew > > The distinction is necessary in the pagewalk when SMAP is enabled. Refer to > Intel SDM Vol 3 "Access Rights" for the exact details. > > Introduce a new pagewalk input, and make use of the new system segment > references in hvmemul_{read,write}(). While modifying those areas, move the > calculation of the appropriate pagewalk input before its first use. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 27 February 2017 14:03 > To: Xen-devel <xen-devel@lists.xen.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich > <JBeulich@suse.com>; Paul Durrant <Paul.Durrant@citrix.com>; George > Dunlap <George.Dunlap@citrix.com>; Tim (Xen.org) <tim@xen.org> > Subject: [PATCH 1/7] x86/hvm: Correctly identify implicit supervisor accesses > > All actions which refer to the active ldt/gdt/idt or task register > (e.g. loading a new segment selector) are known as implicit supervisor > accesses, even when the access originates from user code. > > The distinction is necessary in the pagewalk when SMAP is enabled. Refer to > Intel SDM Vol 3 "Access Rights" for the exact details. > > Introduce a new pagewalk input, and make use of the new system segment > references in hvmemul_{read,write}(). While modifying those areas, move > the > calculation of the appropriate pagewalk input before its first use. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: George Dunlap <george.dunlap@eu.citrix.com> > CC: Tim Deegan <tim@xen.org> > --- > xen/arch/x86/hvm/emulate.c | 18 ++++++++++-------- > xen/arch/x86/mm/guest_walk.c | 4 ++++ > xen/include/asm-x86/processor.h | 1 + > 3 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index f24d289..9b32bca 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -783,6 +783,11 @@ static int __hvmemul_read( > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > int rc; > > + if ( is_x86_system_segment(seg) ) > + pfec |= PFEC_implicit; > + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) > + pfec |= PFEC_user_mode; > + > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr); > if ( rc != X86EMUL_OKAY || !bytes ) > @@ -793,10 +798,6 @@ static int __hvmemul_read( > (vio->mmio_gla == (addr & PAGE_MASK)) ) > return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec, > hvmemul_ctxt, 1); > > - if ( (seg != x86_seg_none) && > - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) > - pfec |= PFEC_user_mode; > - > rc = ((access_type == hvm_access_insn_fetch) ? > hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) : > hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo)); > @@ -893,6 +894,11 @@ static int hvmemul_write( > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > int rc; > > + if ( is_x86_system_segment(seg) ) > + pfec |= PFEC_implicit; > + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) > + pfec |= PFEC_user_mode; > + > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); > if ( rc != X86EMUL_OKAY || !bytes ) > @@ -902,10 +908,6 @@ static int hvmemul_write( > (vio->mmio_gla == (addr & PAGE_MASK)) ) > return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec, > hvmemul_ctxt, 1); > > - if ( (seg != x86_seg_none) && > - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) > - pfec |= PFEC_user_mode; > - > rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo); > > switch ( rc ) > diff --git a/xen/arch/x86/mm/guest_walk.c > b/xen/arch/x86/mm/guest_walk.c > index faaf70c..4f8d0e3 100644 > --- a/xen/arch/x86/mm/guest_walk.c > +++ b/xen/arch/x86/mm/guest_walk.c > @@ -161,6 +161,10 @@ guest_walk_tables(struct vcpu *v, struct > p2m_domain *p2m, > bool_t pse1G = 0, pse2M = 0; > p2m_query_t qt = P2M_ALLOC | P2M_UNSHARE; > > + /* Only implicit supervisor data accesses exist. */ > + ASSERT(!(pfec & PFEC_implicit) || > + !(pfec & (PFEC_insn_fetch|PFEC_user_mode))); > + > perfc_incr(guest_walk); > memset(gw, 0, sizeof(*gw)); > gw->va = va; > diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm- > x86/processor.h > index dda8b83..d3ba8ea 100644 > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -76,6 +76,7 @@ > /* Internally used only flags. */ > #define PFEC_page_paged (1U<<16) > #define PFEC_page_shared (1U<<17) > +#define PFEC_implicit (1U<<18) /* Pagewalk input for ldt/gdt/idt/tr > accesses. */ > > /* Other exception error code values. */ > #define X86_XEC_EXT (_AC(1,U) << 0) > -- > 2.1.4
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index f24d289..9b32bca 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -783,6 +783,11 @@ static int __hvmemul_read( struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; int rc; + if ( is_x86_system_segment(seg) ) + pfec |= PFEC_implicit; + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) + pfec |= PFEC_user_mode; + rc = hvmemul_virtual_to_linear( seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr); if ( rc != X86EMUL_OKAY || !bytes ) @@ -793,10 +798,6 @@ static int __hvmemul_read( (vio->mmio_gla == (addr & PAGE_MASK)) ) return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec, hvmemul_ctxt, 1); - if ( (seg != x86_seg_none) && - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) - pfec |= PFEC_user_mode; - rc = ((access_type == hvm_access_insn_fetch) ? hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) : hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo)); @@ -893,6 +894,11 @@ static int hvmemul_write( struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; int rc; + if ( is_x86_system_segment(seg) ) + pfec |= PFEC_implicit; + else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 ) + pfec |= PFEC_user_mode; + rc = hvmemul_virtual_to_linear( seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); if ( rc != X86EMUL_OKAY || !bytes ) @@ -902,10 +908,6 @@ static int hvmemul_write( (vio->mmio_gla == (addr & PAGE_MASK)) ) return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec, hvmemul_ctxt, 1); - if ( (seg != x86_seg_none) && - (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) ) - pfec |= PFEC_user_mode; - rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo); switch ( rc ) diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c index faaf70c..4f8d0e3 100644 --- a/xen/arch/x86/mm/guest_walk.c +++ b/xen/arch/x86/mm/guest_walk.c @@ -161,6 +161,10 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, bool_t pse1G = 0, pse2M = 0; p2m_query_t qt = P2M_ALLOC | P2M_UNSHARE; + /* Only implicit supervisor data accesses exist. */ + ASSERT(!(pfec & PFEC_implicit) || + !(pfec & (PFEC_insn_fetch|PFEC_user_mode))); + perfc_incr(guest_walk); memset(gw, 0, sizeof(*gw)); gw->va = va; diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index dda8b83..d3ba8ea 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -76,6 +76,7 @@ /* Internally used only flags. */ #define PFEC_page_paged (1U<<16) #define PFEC_page_shared (1U<<17) +#define PFEC_implicit (1U<<18) /* Pagewalk input for ldt/gdt/idt/tr accesses. */ /* Other exception error code values. */ #define X86_XEC_EXT (_AC(1,U) << 0)
All actions which refer to the active ldt/gdt/idt or task register (e.g. loading a new segment selector) are known as implicit supervisor accesses, even when the access originates from user code. The distinction is necessary in the pagewalk when SMAP is enabled. Refer to Intel SDM Vol 3 "Access Rights" for the exact details. Introduce a new pagewalk input, and make use of the new system segment references in hvmemul_{read,write}(). While modifying those areas, move the calculation of the appropriate pagewalk input before its first use. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Paul Durrant <paul.durrant@citrix.com> CC: George Dunlap <george.dunlap@eu.citrix.com> CC: Tim Deegan <tim@xen.org> --- xen/arch/x86/hvm/emulate.c | 18 ++++++++++-------- xen/arch/x86/mm/guest_walk.c | 4 ++++ xen/include/asm-x86/processor.h | 1 + 3 files changed, 15 insertions(+), 8 deletions(-)