executor: fix handling of big-endian bitfields

Currently we apply big-endian-ness and bitfield-ness in the wrong order in copyin.
This leads to totally bogus result. Fix this.

5 files changed, 53 insertions, 43 deletions

diff --git a/executor/common.h b/executor/common.h
index 49a2cbd67..7e855b91c 100644
--- a/executor/common.h
+++ b/executor/common.h
@@ -325,19 +325,10 @@ static int event_timedwait(event_t* ev, uint64 timeout)
 #endif
 
 #if SYZ_EXECUTOR || SYZ_USE_BITMASKS
-#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) = (type)(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;                                         \
-	}
+#define BITMASK(bf_off, bf_len) (((1ull << (bf_len)) - 1) << (bf_off))
+#define STORE_BY_BITMASK(type, htobe, addr, val, bf_off, bf_len)                        \
+	*(type*)(addr) = htobe((htobe(*(type*)(addr)) & ~BITMASK((bf_off), (bf_len))) | \
+			       (((type)(val) << (bf_off)) & BITMASK((bf_off), (bf_len))))
 #endif
 
 #if SYZ_EXECUTOR || SYZ_USE_CHECKSUMS
diff --git a/executor/common_test.h b/executor/common_test.h
index 51b135377..592fafced 100644
--- a/executor/common_test.h
+++ b/executor/common_test.h
@@ -40,7 +40,10 @@ static long syz_compare(long want, long want_len, long got, long got_len)
 		return -1;
 	}
 	if (memcmp((void*)want, (void*)got, want_len)) {
-		debug("syz_compare: data differs\n");
+		debug("syz_compare: data differs, want:\n");
+		debug_dump_data((char*)want, want_len);
+		debug("got:\n");
+		debug_dump_data((char*)got, got_len);
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/executor/defs.h b/executor/defs.h
index 8a28717c5..111355162 100644
--- a/executor/defs.h
+++ b/executor/defs.h
@@ -145,7 +145,7 @@
 
 #if GOARCH_32_fork_shmem
 #define GOARCH "32_fork_shmem"
-#define SYZ_REVISION "4225c1e93671306efa6a41958a6d553aed7e8cf7"
+#define SYZ_REVISION "f0257b726ddd3b09086a9525a4aae0e0d8cfa6af"
 #define SYZ_EXECUTOR_USES_FORK_SERVER 1
 #define SYZ_EXECUTOR_USES_SHMEM 1
 #define SYZ_PAGE_SIZE 4096
@@ -155,7 +155,7 @@
 
 #if GOARCH_32_shmem
 #define GOARCH "32_shmem"
-#define SYZ_REVISION "ae161a1d8e44b101412b6f8d8fdde3a6ce553e55"
+#define SYZ_REVISION "136d60e9280b55ca8a1f24fed877e2f0ae72e348"
 #define SYZ_EXECUTOR_USES_FORK_SERVER 0
 #define SYZ_EXECUTOR_USES_SHMEM 1
 #define SYZ_PAGE_SIZE 8192
@@ -165,7 +165,7 @@
 
 #if GOARCH_64
 #define GOARCH "64"
-#define SYZ_REVISION "6ffded136a7c445ee912402759cc9f71c3add37a"
+#define SYZ_REVISION "82736d421a5d52db6df0775561f1e59cc6cb9014"
 #define SYZ_EXECUTOR_USES_FORK_SERVER 0
 #define SYZ_EXECUTOR_USES_SHMEM 0
 #define SYZ_PAGE_SIZE 4096
@@ -175,7 +175,7 @@
 
 #if GOARCH_64_fork
 #define GOARCH "64_fork"
-#define SYZ_REVISION "ef850b63cd75f943301e586db069812cc63ac259"
+#define SYZ_REVISION "0c64cdd471dfa62b3e34ed221afe8472c9125d38"
 #define SYZ_EXECUTOR_USES_FORK_SERVER 1
 #define SYZ_EXECUTOR_USES_SHMEM 0
 #define SYZ_PAGE_SIZE 8192
diff --git a/executor/executor.cc b/executor/executor.cc
index 68c981552..6569326d3 100644
--- a/executor/executor.cc
+++ b/executor/executor.cc
@@ -295,6 +295,7 @@ static uint64 read_input(uint64** input_posp, bool peek = false);
 static uint64 read_arg(uint64** input_posp);
 static uint64 read_const_arg(uint64** input_posp, uint64* size_p, uint64* bf, uint64* bf_off_p, uint64* bf_len_p);
 static uint64 read_result(uint64** input_posp);
+static uint64 swap(uint64 v, uint64 size, uint64 bf);
 static void copyin(char* addr, uint64 val, uint64 size, uint64 bf, uint64 bf_off, uint64 bf_len);
 static bool copyout(char* addr, uint64 size, uint64* res);
 static void setup_control_pipes();
@@ -1026,24 +1027,37 @@ static bool dedup(uint32 sig)
 }
 #endif
 
+template <typename T>
+void copyin_int(char* addr, uint64 val, uint64 bf, uint64 bf_off, uint64 bf_len)
+{
+	if (bf_off == 0 && bf_len == 0) {
+		*(T*)addr = swap(val, sizeof(T), bf);
+		return;
+	}
+	T x = swap(*(T*)addr, sizeof(T), bf);
+	x = (x & ~BITMASK(bf_off, bf_len)) | ((val << bf_off) & BITMASK(bf_off, bf_len));
+	*(T*)addr = swap(x, sizeof(T), bf);
+}
+
 void copyin(char* addr, uint64 val, uint64 size, uint64 bf, uint64 bf_off, uint64 bf_len)
 {
-	if (bf != binary_format_native && (bf_off != 0 || bf_len != 0))
+	if (bf != binary_format_native && bf != binary_format_bigendian && (bf_off != 0 || bf_len != 0))
 		fail("bitmask for string format %llu/%llu", bf_off, bf_len);
 	switch (bf) {
 	case binary_format_native:
+	case binary_format_bigendian:
 		NONFAILING(switch (size) {
 			case 1:
-				STORE_BY_BITMASK(uint8, addr, val, bf_off, bf_len);
+				copyin_int<uint8>(addr, val, bf, bf_off, bf_len);
 				break;
 			case 2:
-				STORE_BY_BITMASK(uint16, addr, val, bf_off, bf_len);
+				copyin_int<uint16>(addr, val, bf, bf_off, bf_len);
 				break;
 			case 4:
-				STORE_BY_BITMASK(uint32, addr, val, bf_off, bf_len);
+				copyin_int<uint32>(addr, val, bf, bf_off, bf_len);
 				break;
 			case 8:
-				STORE_BY_BITMASK(uint64, addr, val, bf_off, bf_len);
+				copyin_int<uint64>(addr, val, bf, bf_off, bf_len);
 				break;
 			default:
 				fail("copyin: bad argument size %llu", size);
@@ -1099,11 +1113,11 @@ uint64 read_arg(uint64** input_posp)
 	case arg_const: {
 		uint64 size, bf, bf_off, bf_len;
 		uint64 val = read_const_arg(input_posp, &size, &bf, &bf_off, &bf_len);
-		if (bf != binary_format_native)
+		if (bf != binary_format_native && bf != binary_format_bigendian)
 			fail("bad argument binary format %llu", bf);
 		if (bf_off != 0 || bf_len != 0)
 			fail("bad argument bitfield %llu/%llu", bf_off, bf_len);
-		return val;
+		return swap(val, size, bf);
 	}
 	case arg_result: {
 		uint64 meta = read_input(input_posp);
@@ -1117,6 +1131,24 @@ uint64 read_arg(uint64** input_posp)
 	}
 }
 
+uint64 swap(uint64 v, uint64 size, uint64 bf)
+{
+	if (bf == binary_format_native)
+		return v;
+	if (bf != binary_format_bigendian)
+		fail("bad binary format in swap: %llu", bf);
+	switch (size) {
+	case 2:
+		return htobe16(v);
+	case 4:
+		return htobe32(v);
+	case 8:
+		return htobe64(v);
+	default:
+		fail("bad big-endian int size %llu", size);
+	}
+}
+
 uint64 read_const_arg(uint64** input_posp, uint64* size_p, uint64* bf_p, uint64* bf_off_p, uint64* bf_len_p)
 {
 	uint64 meta = read_input(input_posp);
@@ -1127,22 +1159,6 @@ uint64 read_const_arg(uint64** input_posp, uint64* size_p, uint64* bf_p, uint64*
 	*bf_len_p = (meta >> 24) & 0xff;
 	uint64 pid_stride = meta >> 32;
 	val += pid_stride * procid;
-	if (bf == binary_format_bigendian) {
-		bf = binary_format_native;
-		switch (*size_p) {
-		case 2:
-			val = htobe16(val);
-			break;
-		case 4:
-			val = htobe32(val);
-			break;
-		case 8:
-			val = htobe64(val);
-			break;
-		default:
-			fail("bad big-endian int size %llu", *size_p);
-		}
-	}
 	*bf_p = bf;
 	return val;
 }
diff --git a/executor/test.h b/executor/test.h
index a5bc8e8f2..c39f611d1 100644
--- a/executor/test.h
+++ b/executor/test.h
@@ -8,7 +8,7 @@
 static int test_copyin()
 {
 	static uint16 buf[3];
-	STORE_BY_BITMASK(uint16, &buf[1], 0x1234, 0, 0);
+	STORE_BY_BITMASK(uint16, , &buf[1], 0x1234, 0, 16);
 	unsigned char x[sizeof(buf)];
 	memcpy(x, buf, sizeof(x));
 	if (x[0] != 0 || x[1] != 0 ||
@@ -18,7 +18,7 @@ static int test_copyin()
 		       x[0], x[1], x[2], x[3], x[4], x[5]);
 		return 1;
 	}
-	STORE_BY_BITMASK(uint16, &buf[1], 0x555a, 5, 4);
+	STORE_BY_BITMASK(uint16, , &buf[1], 0x555a, 5, 4);
 	memcpy(x, buf, sizeof(x));
 	if (x[0] != 0 || x[1] != 0 ||
 	    x[2] != 0x54 || x[3] != 0x13 ||