Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12:Update
xen.10697
5b6d84ac-x86-fix-improve-vlapic-read-write.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 5b6d84ac-x86-fix-improve-vlapic-read-write.patch of Package xen.10697
# Commit b6f43c14cef3af8477a9eca4efab87dd150a2885 # Date 2018-08-10 13:27:24 +0100 # Author Andrew Cooper <andrew.cooper3@citrix.com> # Committer Andrew Cooper <andrew.cooper3@citrix.com> x86/vlapic: Bugfixes and improvements to vlapic_{read,write}() Firstly, there is no 'offset' boundary check on the non-32-bit write path before the call to vlapic_read_aligned(), which allows an attacker to read beyond the end of vlapic->regs->data[], which is only 1024 bytes long. However, as the backing memory is a domheap page, and misaligned accesses get chunked down to single bytes across page boundaries, I can't spot any XSA-worthy problems which occur from the overrun. On real hardware, bad accesses don't instantly crash the machine. Their behaviour is undefined, but the domain_crash() prohibits sensible testing. Behave more like other x86 MMIO and terminate bad accesses with appropriate defaults. While making these changes, clean up and simplify the the smaller-access handling. In particular, avoid pointer based mechansims for 1/2-byte reads so as to avoid forcing the value to be spilled to the stack. add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-175 (-175) function old new delta vlapic_read 211 142 -69 vlapic_write 304 198 -106 Finally, there are a plethora of read/write functions in the vlapic namespace, so rename these to vlapic_mmio_{read,write}() to make their purpose more clear. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau MonnĂ© <roger.pau@citrix.com> --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -587,59 +587,39 @@ static void vlapic_read_aligned( } } -static int vlapic_read( - struct vcpu *v, unsigned long address, - unsigned long len, unsigned long *pval) +static int vlapic_mmio_read(struct vcpu *v, unsigned long address, + unsigned long len, unsigned long *pval) { - unsigned int alignment; - unsigned int tmp; - unsigned long result = 0; struct vlapic *vlapic = vcpu_vlapic(v); unsigned int offset = address - vlapic_base_address(vlapic); + unsigned int alignment = offset & 0xf, result = 0; - if ( offset > (APIC_TDCR + 0x3) ) - goto out; - - alignment = offset & 0x3; - - vlapic_read_aligned(vlapic, offset & ~0x3, &tmp); - switch ( len ) + /* + * APIC registers are 32-bit values, aligned on 128-bit boundaries, and + * should be accessed with 32-bit wide loads. + * + * Some processors support smaller accesses, so we allow any access which + * fully fits within the 32-bit register. + */ + if ( (alignment + len) <= 4 && offset <= (APIC_TDCR + 3) ) { - case 1: - result = *((unsigned char *)&tmp + alignment); - break; + unsigned int reg; - case 2: - if ( alignment == 3 ) - goto unaligned_exit_and_crash; - result = *(unsigned short *)((unsigned char *)&tmp + alignment); - break; + vlapic_read_aligned(vlapic, offset & ~0xf, ®); - case 4: - if ( alignment != 0 ) - goto unaligned_exit_and_crash; - result = *(unsigned int *)((unsigned char *)&tmp + alignment); - break; + switch ( len ) + { + case 1: result = (uint8_t) (reg >> (alignment * 8)); break; + case 2: result = (uint16_t)(reg >> (alignment * 8)); break; + case 4: result = reg; break; + } - default: - gdprintk(XENLOG_ERR, "Local APIC read with len=%#lx, " - "should be 4 instead.\n", len); - goto exit_and_crash; + HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, " + "and the result is %#x", offset, len, result); } - HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, " - "and the result is %#lx", offset, len, result); - - out: *pval = result; return X86EMUL_OKAY; - - unaligned_exit_and_crash: - gdprintk(XENLOG_ERR, "Unaligned LAPIC read len=%#lx at offset=%#x.\n", - len, offset); - exit_and_crash: - domain_crash(v->domain); - return X86EMUL_OKAY; } int hvm_x2apic_msr_read(struct vcpu *v, unsigned int msr, uint64_t *msr_content) @@ -835,12 +815,14 @@ static int vlapic_reg_write(struct vcpu return rc; } -static int vlapic_write(struct vcpu *v, unsigned long address, - unsigned long len, unsigned long val) +static int vlapic_mmio_write(struct vcpu *v, unsigned long address, + unsigned long len, unsigned long val) { struct vlapic *vlapic = vcpu_vlapic(v); unsigned int offset = address - vlapic_base_address(vlapic); - int rc = X86EMUL_OKAY; + unsigned int alignment = offset & 0xf; + + offset &= ~0xf; if ( offset != 0xb0 ) HVM_DBG_LOG(DBG_LEVEL_VLAPIC, @@ -848,53 +830,41 @@ static int vlapic_write(struct vcpu *v, offset, len, val); /* - * According to the IA32 Manual, all accesses should be 32 bits. - * Some OSes do 8- or 16-byte accesses, however. + * APIC registers are 32-bit values, aligned on 128-bit boundaries, and + * should be accessed with 32-bit wide stores. + * + * Some processors support smaller accesses, so we allow any access which + * fully fits within the 32-bit register. */ val = (uint32_t)val; - if ( len != 4 ) + if ( (alignment + len) <= 4 && offset <= APIC_TDCR ) { - unsigned int tmp; - unsigned char alignment; - - gdprintk(XENLOG_INFO, "Notice: Local APIC write with len = %lx\n",len); - - alignment = offset & 0x3; - (void)vlapic_read_aligned(vlapic, offset & ~0x3, &tmp); - - switch ( len ) + if ( unlikely(len < 4) ) { - case 1: - val = ((tmp & ~(0xff << (8*alignment))) | - ((val & 0xff) << (8*alignment))); - break; + unsigned int reg; - case 2: - if ( alignment & 1 ) - goto unaligned_exit_and_crash; - val = ((tmp & ~(0xffff << (8*alignment))) | - ((val & 0xffff) << (8*alignment))); - break; + vlapic_read_aligned(vlapic, offset, ®); - default: - gdprintk(XENLOG_ERR, "Local APIC write with len = %lx, " - "should be 4 instead\n", len); - goto exit_and_crash; - } - } - else if ( (offset & 0x3) != 0 ) - goto unaligned_exit_and_crash; + alignment *= 8; - offset &= ~0x3; + switch ( len ) + { + case 1: + val = ((reg & ~(0xffU << alignment)) | + ((val & 0xff) << alignment)); + break; + + case 2: + val = ((reg & ~(0xffffU << alignment)) | + ((val & 0xffff) << alignment)); + break; + } + } - return vlapic_reg_write(v, offset, val); + vlapic_reg_write(v, offset, val); + } - unaligned_exit_and_crash: - gdprintk(XENLOG_ERR, "Unaligned LAPIC write len=%#lx at offset=%#x.\n", - len, offset); - exit_and_crash: - domain_crash(v->domain); - return rc; + return X86EMUL_OKAY; } int vlapic_apicv_write(struct vcpu *v, unsigned int offset) @@ -1002,8 +972,8 @@ static int vlapic_range(struct vcpu *v, const struct hvm_mmio_handler vlapic_mmio_handler = { .check_handler = vlapic_range, - .read_handler = vlapic_read, - .write_handler = vlapic_write + .read_handler = vlapic_mmio_read, + .write_handler = vlapic_mmio_write, }; static void set_x2apic_id(struct vlapic *vlapic)
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
