From 1ac75f06add54089e4910ecb6a97198336155c63 Mon Sep 17 00:00:00 2001
From: Dmitry Vyukov <dvyukov@google.com>
Date: Tue, 17 Jan 2017 19:04:37 +0100
Subject: executor: fix copyin of values

Currently non-bitfield values are copied incorrectly.
Probably all turned into zeros or something.
Fix that. Add test.
---
 executor/common.h     | 24 +++++++++++++-----------
 executor/test.go      |  5 +++++
 executor/test_kvm.cc  | 16 ++++++++++++++++
 executor/test_test.go | 10 ++++++++++
 4 files changed, 44 insertions(+), 11 deletions(-)

(limited to 'executor')

diff --git a/executor/common.h b/executor/common.h
index 40b2bf8d0..5c5bb1032 100644
--- a/executor/common.h
+++ b/executor/common.h
@@ -158,17 +158,19 @@ static void install_segv_handler()
 		__atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST); \
 	}
 
-#define BITMASK_LEN(type, bf_len) (type)((1ul << bf_len) - 1)
-
-#define BITMASK_LEN_OFF(type, bf_off, bf_len) (type)(BITMASK_LEN(type, bf_len) << bf_off)
-
-#define STORE_BY_BITMASK(type, addr, val, bf_off, bf_len)                     \
-	do {                                                                  \
-		type new_val = *(type*)addr;                                  \
-		new_val &= ~BITMASK_LEN_OFF(type, bf_off, bf_len);            \
-		new_val |= ((type)val & BITMASK_LEN(type, bf_len)) << bf_off; \
-		*(type*)addr = new_val;                                       \
-	} while (0)
+#define BITMASK_LEN(type, bf_len) (type)((1ull << (bf_len)) - 1)
+
+#define BITMASK_LEN_OFF(type, bf_off, bf_len) (type)(BITMASK_LEN(type, (bf_len)) << (bf_off))
+
+#define STORE_BY_BITMASK(type, addr, val, bf_off, bf_len)                         \
+	if ((bf_off) == 0 && (bf_len) == 0) {                                     \
+		*(type*)(addr) = (val);                                           \
+	} else {                                                                  \
+		type new_val = *(type*)(addr);                                    \
+		new_val &= ~BITMASK_LEN_OFF(type, (bf_off), (bf_len));            \
+		new_val |= ((type)(val)&BITMASK_LEN(type, (bf_len))) << (bf_off); \
+		*(type*)(addr) = new_val;                                         \
+	}
 
 #ifdef __NR_syz_emit_ethernet
 static void vsnprintf_check(char* str, size_t size, const char* format, va_list args)
diff --git a/executor/test.go b/executor/test.go
index 09a1c130b..e9709b8d0 100644
--- a/executor/test.go
+++ b/executor/test.go
@@ -5,9 +5,14 @@
 
 package executor
 
+// int test_copyin();
 // int test_kvm();
 import "C"
 
+func testCopyin() int {
+	return int(C.test_copyin())
+}
+
 func testKVM() int {
 	return int(C.test_kvm())
 }
diff --git a/executor/test_kvm.cc b/executor/test_kvm.cc
index 4d990dc19..6c76b5e78 100644
--- a/executor/test_kvm.cc
+++ b/executor/test_kvm.cc
@@ -8,6 +8,22 @@
 
 #include <sys/utsname.h>
 
+extern "C" int test_copyin()
+{
+	unsigned char x[4] = {};
+	STORE_BY_BITMASK(uint16_t, &x[1], 0x1234, 0, 0);
+	if (x[0] != 0 || x[1] != 0x34 || x[2] != 0x12 || x[3] != 0) {
+		printf("bad result of STORE_BY_BITMASK(0, 0): %x %x %x %x\n", x[0], x[1], x[2], x[3]);
+		return 1;
+	}
+	STORE_BY_BITMASK(uint16_t, &x[1], 0x555a, 5, 4);
+	if (x[0] != 0 || x[1] != 0x54 || x[2] != 0x13 || x[3] != 0) {
+		printf("bad result of STORE_BY_BITMASK(7, 3): %x %x %x %x\n", x[0], x[1], x[2], x[3]);
+		return 1;
+	}
+	return 0;
+}
+
 static unsigned host_kernel_version();
 static void dump_cpu_state(int cpufd, char* vm_mem);
 
diff --git a/executor/test_test.go b/executor/test_test.go
index 86379b93b..f197dbf51 100644
--- a/executor/test_test.go
+++ b/executor/test_test.go
@@ -5,6 +5,16 @@ package executor
 
 import "testing"
 
+func TestCopyin(t *testing.T) {
+	switch res := testCopyin(); {
+	case res < 0:
+		t.Skip()
+	case res > 0:
+		t.Fail()
+	default:
+	}
+}
+
 func TestKVM(t *testing.T) {
 	switch res := testKVM(); {
 	case res < 0:
-- 
cgit mrf-deployment