From 566fb0edfc6d88efcd161c912c7db025b8c173ce Mon Sep 17 00:00:00 2001 From: ec- Date: Wed, 15 Mar 2017 11:42:58 +0200 Subject: [PATCH] Allow unaligned load/store in QVM interpreter/x86 compiler constructions like (dataMask & ~3) was used to protect against out-of-bound load/store when address is 4-byte closer to dataMask but at the same time it effectively cut low address bits for ALL load/store operations which is totally wrong in terms of conformance to ALLOWED (i.e. generated by q3lcc from C sources) low-level operations like packed binary data parsing --- code/qcommon/vm.c | 8 +++++--- code/qcommon/vm_interpreted.c | 14 +++++++------- code/qcommon/vm_local.h | 1 + code/qcommon/vm_x86.c | 8 ++++---- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/code/qcommon/vm.c b/code/qcommon/vm.c index e8818a6c..dcf5117e 100644 --- a/code/qcommon/vm.c +++ b/code/qcommon/vm.c @@ -451,13 +451,15 @@ vmHeader_t *VM_LoadQVM( vm_t *vm, qboolean alloc, qboolean unpure) if(alloc) { // allocate zero filled space for initialized and uninitialized data - vm->dataBase = Hunk_Alloc(dataLength, h_high); + // leave some space beyound data mask so we can secure all mask operations + vm->dataAlloc = dataLength + 4; + vm->dataBase = Hunk_Alloc(vm->dataAlloc, h_high); vm->dataMask = dataLength - 1; } else { // clear the data, but make sure we're not clearing more than allocated - if(vm->dataMask + 1 != dataLength) + if(vm->dataAlloc != dataLength + 4) { VM_Free(vm); FS_FreeFile(header.v); @@ -467,7 +469,7 @@ vmHeader_t *VM_LoadQVM( vm_t *vm, qboolean alloc, qboolean unpure) return NULL; } - Com_Memset(vm->dataBase, 0, dataLength); + Com_Memset(vm->dataBase, 0, vm->dataAlloc); } // copy the intialized data diff --git a/code/qcommon/vm_interpreted.c b/code/qcommon/vm_interpreted.c index f630d053..cb86a08d 100644 --- a/code/qcommon/vm_interpreted.c +++ b/code/qcommon/vm_interpreted.c @@ -436,31 +436,31 @@ nextInstruction2: return 0; } #endif - r0 = opStack[opStackOfs] = *(int *) &image[r0 & dataMask & ~3 ]; + r0 = opStack[opStackOfs] = *(int *) &image[ r0 & dataMask ]; goto nextInstruction2; case OP_LOAD2: - r0 = opStack[opStackOfs] = *(unsigned short *)&image[ r0&dataMask&~1 ]; + r0 = opStack[opStackOfs] = *(unsigned short *)&image[ r0 & dataMask ]; goto nextInstruction2; case OP_LOAD1: - r0 = opStack[opStackOfs] = image[ r0&dataMask ]; + r0 = opStack[opStackOfs] = image[ r0 & dataMask ]; goto nextInstruction2; case OP_STORE4: - *(int *)&image[ r1&(dataMask & ~3) ] = r0; + *(int *)&image[ r1 & dataMask ] = r0; opStackOfs -= 2; goto nextInstruction; case OP_STORE2: - *(short *)&image[ r1&(dataMask & ~1) ] = r0; + *(short *)&image[ r1 & dataMask ] = r0; opStackOfs -= 2; goto nextInstruction; case OP_STORE1: - image[ r1&dataMask ] = r0; + image[ r1 & dataMask ] = r0; opStackOfs -= 2; goto nextInstruction; case OP_ARG: // single byte offset from programStack - *(int *)&image[ (codeImage[programCounter] + programStack)&dataMask&~3 ] = r0; + *(int *)&image[ (codeImage[programCounter] + programStack) & dataMask ] = r0; opStackOfs--; programCounter += 1; goto nextInstruction; diff --git a/code/qcommon/vm_local.h b/code/qcommon/vm_local.h index 76b1a4b1..07e89675 100644 --- a/code/qcommon/vm_local.h +++ b/code/qcommon/vm_local.h @@ -170,6 +170,7 @@ struct vm_s { byte *dataBase; int dataMask; + int dataAlloc; // actually allocated int stackBottom; // if programStack < stackBottom, error diff --git a/code/qcommon/vm_x86.c b/code/qcommon/vm_x86.c index 9ee26799..2435a6e9 100644 --- a/code/qcommon/vm_x86.c +++ b/code/qcommon/vm_x86.c @@ -790,7 +790,7 @@ qboolean ConstOptimize(vm_t *vm, int callProcOfsSyscall) return qtrue; case OP_STORE4: - EmitMovEAXStack(vm, (vm->dataMask & ~3)); + EmitMovEAXStack(vm, vm->dataMask); #if idx64 EmitRexString(0x41, "C7 04 01"); // mov dword ptr [r9 + eax], 0x12345678 Emit4(Constant4()); @@ -805,7 +805,7 @@ qboolean ConstOptimize(vm_t *vm, int callProcOfsSyscall) return qtrue; case OP_STORE2: - EmitMovEAXStack(vm, (vm->dataMask & ~1)); + EmitMovEAXStack(vm, vm->dataMask); #if idx64 Emit1(0x66); // mov word ptr [r9 + eax], 0x1234 EmitRexString(0x41, "C7 04 01"); @@ -1377,7 +1377,7 @@ void VM_Compile(vm_t *vm, vmHeader_t *header) case OP_STORE4: EmitMovEAXStack(vm, 0); EmitString("8B 54 9F FC"); // mov edx, dword ptr -4[edi + ebx * 4] - MASK_REG("E2", vm->dataMask & ~3); // and edx, 0x12345678 + MASK_REG("E2", vm->dataMask); // and edx, 0x12345678 #if idx64 EmitRexString(0x41, "89 04 11"); // mov dword ptr [r9 + edx], eax #else @@ -1389,7 +1389,7 @@ void VM_Compile(vm_t *vm, vmHeader_t *header) case OP_STORE2: EmitMovEAXStack(vm, 0); EmitString("8B 54 9F FC"); // mov edx, dword ptr -4[edi + ebx * 4] - MASK_REG("E2", vm->dataMask & ~1); // and edx, 0x12345678 + MASK_REG("E2", vm->dataMask); // and edx, 0x12345678 #if idx64 Emit1(0x66); // mov word ptr [r9 + edx], eax EmitRexString(0x41, "89 04 11");