Ping#3: [PATCH v3] x86/HVM: don't #GP/#SS on wrapping virt->linear translations

["Jan Beulich" <JBeulich@suse.com>]

>>> On 25.08.17 at 16:59,  wrote:
>>>> On 10.08.17 at 09:19, <JBeulich@suse.com> wrote:
> >>>> On 10.07.17 at 12:39, <JBeulich@suse.com> wrote:
> >> Real hardware wraps silently in most cases, so we should behave the
> >> same. Also split real and VM86 mode handling, as the latter really
> >> ought to have limit checks applied.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> v3: Restore 32-bit wrap check for AMD.
> >> v2: Extend to non-64-bit modes. Reduce 64-bit check to a single
> >>     is_canonical_address() invocation.

Same here - I think I've been carrying this for long enough.

Jan

> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -2416,16 +2416,21 @@ bool_t hvm_virtual_to_linear_addr(
> >>       */
> >>      ASSERT(seg < x86_seg_none);
> >>  
> >> -    if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
> >> -         (guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) )
> >> +    if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) )
> >>      {
> >>          /*
> >> -         * REAL/VM86 MODE: Don't bother with segment access checks.
> >> +         * REAL MODE: Don't bother with segment access checks.
> >>           * Certain of them are not done in native real mode anyway.
> >>           */
> >>          addr = (uint32_t)(addr + reg->base);
> >> -        last_byte = (uint32_t)addr + bytes - !!bytes;
> >> -        if ( last_byte < addr )
> >> +    }
> >> +    else if ( (guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) &&
> >> +              is_x86_user_segment(seg) )
> >> +    {
> >> +        /* VM86 MODE: Fixed 64k limits on all user segments. */
> >> +        addr = (uint32_t)(addr + reg->base);
> >> +        last_byte = (uint32_t)offset + bytes - !!bytes;
> >> +        if ( max(offset, last_byte) >> 16 )
> >>              goto out;
> >>      }
> >>      else if ( hvm_long_mode_active(curr) &&
> >> @@ -2447,8 +2452,7 @@ bool_t hvm_virtual_to_linear_addr(
> >>              addr += reg->base;
> >>  
> >>          last_byte = addr + bytes - !!bytes;
> >> -        if ( !is_canonical_address(addr) || last_byte < addr ||
> >> -             !is_canonical_address(last_byte) )
> >> +        if ( !is_canonical_address((long)addr < 0 ? addr : last_byte) )
> >>              goto out;
> >>      }
> >>      else
> >> @@ -2498,8 +2502,11 @@ bool_t hvm_virtual_to_linear_addr(
> >>              if ( (offset <= reg->limit) || (last_byte < offset) )
> >>                  goto out;
> >>          }
> >> -        else if ( (last_byte > reg->limit) || (last_byte < offset) )
> >> -            goto out; /* last byte is beyond limit or wraps 0xFFFFFFFF */
> >> +        else if ( last_byte > reg->limit )
> >> +            goto out; /* last byte is beyond limit */
> >> +        else if ( last_byte < offset &&
> >> +                  curr->domain->arch.cpuid->x86_vendor == X86_VENDOR_AMD )
> >> +            goto out; /* access wraps */
> >>      }
> >>  
> >>      /* All checks ok. */
> 
> 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel