executor: prevent executor from messing with output region

When comparisons are enabled fuzzer somehow manages to discover
the output region and corrupt it. It seems to fetch the address
from some memory operations (mmap/munmap).

Don't leak the output region address.

executor/executor.h

@@ -26,6 +26,7 @@ const int kOutPipeFd = 251; // remapped from stdout
 
 const int kMaxInput = 2 << 20;
 const int kMaxOutput = 16 << 20;
+const int kCoverSize = 64 << 10;
 const int kMaxArgs = 9;
 const int kMaxThreads = 16;
 const int kMaxCommands = 16 << 10;
@@ -164,9 +165,8 @@ struct kcov_comparison_t {
 	uint64_t arg2;
 	uint64_t pc;
 
-	// Writes the structure using the write_one function for each field.
-	// Inspired by write_output() function.
-	void write(uint32_t* (*write_one)(uint32_t));
+	bool ignore() const;
+	void write();
 	bool operator==(const struct kcov_comparison_t& other) const;
 	bool operator<(const struct kcov_comparison_t& other) const;
 };
@@ -520,13 +520,19 @@ void handle_completion(thread_t* th)
 
 		if (flag_collect_comps) {
 			// Collect only the comparisons
-			comps_size = th->cover_size;
+			uint32_t ncomps = th->cover_size;
 			kcov_comparison_t* start = (kcov_comparison_t*)th->cover_data;
-			kcov_comparison_t* end = start + comps_size;
+			kcov_comparison_t* end = start + ncomps;
+			if ((uint64_t*)end >= th->cover_data + kCoverSize)
+				fail("too many comparisons %u", ncomps);
 			std::sort(start, end);
-			comps_size = std::unique(start, end) - start;
-			for (uint32_t i = 0; i < comps_size; ++i)
-				start[i].write(write_output);
+			ncomps = std::unique(start, end) - start;
+			for (uint32_t i = 0; i < ncomps; ++i) {
+				if (start[i].ignore())
+					continue;
+				comps_size++;
+				start[i].write();
+			}
 		} else {
 			// Write out feedback signals.
 			// Currently it is code edges computed as xor of
@@ -761,10 +767,10 @@ uint64_t read_input(uint64_t** input_posp, bool peek)
 	return *input_pos;
 }
 
-void kcov_comparison_t::write(uint32_t* (*write_one)(uint32_t))
+void kcov_comparison_t::write()
 {
 	// Write order: type arg1 arg2 pc.
-	write_one((uint32_t)type);
+	write_output((uint32_t)type);
 
 	// KCOV converts all arguments of size x first to uintx_t and then to
 	// uint64_t. We want to properly extend signed values, e.g we want
@@ -788,15 +794,15 @@ void kcov_comparison_t::write(uint32_t* (*write_one)(uint32_t))
 	}
 	bool is_size_8 = (type & KCOV_CMP_SIZE_MASK) == KCOV_CMP_SIZE8;
 	if (!is_size_8) {
-		write_one((uint32_t)arg1);
-		write_one((uint32_t)arg2);
+		write_output((uint32_t)arg1);
+		write_output((uint32_t)arg2);
 		return;
 	}
 	// If we have 64 bits arguments then write them in Little-endian.
-	write_one((uint32_t)(arg1 & 0xFFFFFFFF));
-	write_one((uint32_t)(arg1 >> 32));
-	write_one((uint32_t)(arg2 & 0xFFFFFFFF));
-	write_one((uint32_t)(arg2 >> 32));
+	write_output((uint32_t)(arg1 & 0xFFFFFFFF));
+	write_output((uint32_t)(arg1 >> 32));
+	write_output((uint32_t)(arg2 & 0xFFFFFFFF));
+	write_output((uint32_t)(arg2 >> 32));
 }
 
 bool kcov_comparison_t::operator==(const struct kcov_comparison_t& other) const

executor/executor_akaros.cc

@@ -98,3 +98,8 @@ uint32_t* write_output(uint32_t v)
 void write_completed(uint32_t completed)
 {
 }
+
+bool kcov_comparison_t::ignore() const
+{
+	return false;
+}

executor/executor_freebsd.cc

@@ -166,3 +166,8 @@ void write_completed(uint32_t completed)
 {
 	__atomic_store_n(output_data, completed, __ATOMIC_RELEASE);
 }
+
+bool kcov_comparison_t::ignore() const
+{
+	return false;
+}

executor/executor_fuchsia.cc

@@ -61,3 +61,8 @@ uint32_t* write_output(uint32_t v)
 void write_completed(uint32_t completed)
 {
 }
+
+bool kcov_comparison_t::ignore() const
+{
+	return false;
+}

executor/executor_linux.cc

@@ -34,8 +34,8 @@ const unsigned long KCOV_TRACE_CMP = 1;
 
 const int kInFd = 3;
 const int kOutFd = 4;
-const int kCoverSize = 64 << 10;
-const int kPageSize = 4 << 10;
+
+void* const kOutputDataAddr = (void*)0x1bdbc20000ull;
 
 uint32_t* output_data;
 uint32_t* output_pos;
@@ -54,7 +54,6 @@ int main(int argc, char** argv)
 	// If it is corrupted ipc package will fail to parse its contents and panic.
 	// But fuzzer constantly invents new ways of how to currupt the region,
 	// so we map the region at a (hopefully) hard to guess address surrounded by unmapped pages.
-	void* const kOutputDataAddr = (void*)0x1ddbc20000;
 	output_data = (uint32_t*)mmap(kOutputDataAddr, kMaxOutput, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, kOutFd, 0);
 	if (output_data != kOutputDataAddr)
 		fail("mmap of output file failed");
@@ -292,3 +291,37 @@ void write_completed(uint32_t completed)
 {
 	__atomic_store_n(output_data, completed, __ATOMIC_RELEASE);
 }
+
+bool kcov_comparison_t::ignore() const
+{
+	// Comparisons with 0 are not interesting, fuzzer should be able to guess 0's without help.
+	if (arg1 == 0 && (arg2 == 0 || (type & KCOV_CMP_CONST)))
+		return true;
+	if ((type & KCOV_CMP_SIZE_MASK) == KCOV_CMP_SIZE8) {
+		// This can be a pointer (assuming 64-bit kernel).
+		// First of all, we want avert fuzzer from our output region.
+		// Without this fuzzer manages to discover and corrupt it.
+		uint64_t out_start = (uint64_t)kOutputDataAddr;
+		uint64_t out_end = out_start + kMaxOutput;
+		if (arg1 >= out_start && arg1 <= out_end)
+			return true;
+		if (arg2 >= out_start && arg2 <= out_end)
+			return true;
+#if defined(__i386__) || defined(__x86_64__)
+		// Filter out kernel physical memory addresses.
+		// These are internal kernel comparisons and should not be interesting.
+		// The range covers first 1TB of physical mapping.
+		uint64_t kmem_start = (uint64_t)0xffff880000000000ull;
+		uint64_t kmem_end = (uint64_t)0xffff890000000000ull;
+		bool kptr1 = arg1 >= kmem_start && arg1 <= kmem_end;
+		bool kptr2 = arg2 >= kmem_start && arg2 <= kmem_end;
+		if (kptr1 && kptr2)
+			return true;
+		if (kptr1 && arg2 == 0)
+			return true;
+		if (kptr2 && arg1 == 0)
+			return true;
+#endif
+	}
+	return false;
+}

executor/executor_windows.cc

@@ -63,3 +63,8 @@ uint32_t* write_output(uint32_t v)
 void write_completed(uint32_t completed)
 {
 }
+
+bool kcov_comparison_t::ignore() const
+{
+	return false;
+}