sandbox: fixes

Signed-off-by: Ahmed Samy <asamy@protonmail.com>

bitmap.h

@@ -41,7 +41,7 @@ typedef unsigned long bitmap_t;
 
 #define BITMAP_BITS			(sizeof(bitmap_t) * CHAR_BIT)
 #define DECLARE_BITMAP(name, bits)	\
-	unsigned long name[BITMAP_BITS * bits]
+	bitmap_t name[BITMAP_BITS * bits]
 
 static inline unsigned long pos_bit(unsigned long pos)
 {

compiler.h

@@ -77,7 +77,9 @@ typedef signed long long intptr_t;
 #define container_of(address, type, field)	CONTAINING_RECORD(address, type, field)
 /* OTOH - MSVC does not have typeof.  Hack it.  */
 #define container_off_var(var, member)			\
-	((const char *)&(var)->member - (const char *)(var))
+	((char *)&(var)->member - (char *)(var))
+#define container_of_var(ptr, var, member)	\
+	((char *)ptr - container_off_var(var, member))
 
 #ifndef UM
 #include "list.h"

ksm.h

@@ -96,11 +96,11 @@
 #define vpid_nr()			(cpu_nr() + 1)
 #ifdef __linux__
 #define proc_name()			current->comm
-#define proc_pid()			current->pid
+#define proc_id()			current->pid
 #else
 #define current				PsGetCurrentProcess()
 #define proc_name()			PsGetProcessImageFileName(current)
-#define proc_pid()			PsGetProcessId(current)
+#define proc_id()			PsGetProcessId(current)
 #endif
 
 #ifdef ENABLE_PRINT
@@ -518,9 +518,14 @@ extern int ksm_free_idt(unsigned n);
 extern bool ksm_write_virt(struct vcpu *vcpu, u64 gva, const u8 *data, size_t len);
 extern bool ksm_read_virt(struct vcpu *vcpu, u64 gva, u8 *data, size_t len);
 
+static inline struct vcpu *ksm_cpu_at(struct ksm *k, int cpu)
+{
+	return &k->vcpu_list[cpu];
+}
+
 static inline struct vcpu *ksm_cpu(struct ksm *k)
 {
-	return &k->vcpu_list[cpu_nr()];
+	return ksm_cpu_at(k, cpu_nr());
 }
 
 static inline struct vcpu *ksm_current_cpu(void)
@@ -592,7 +597,7 @@ typedef HANDLE pid_t;
 extern int ksm_sandbox_init(struct ksm *k);
 extern int ksm_sandbox_exit(struct ksm *k);
 extern bool ksm_sandbox_handle_ept(struct ept *ept, int dpl, u64 gpa,
-				   u64 gva, u16 curr, u8 ar, u8 ac,
+				   u64 gva, u64 cr3, u16 curr, u8 ar, u8 ac,
 				   bool *invd, u16 *eptp_switch);
 extern void ksm_sandbox_handle_cr3(struct vcpu *vcpu, u64 cr3);
 extern int ksm_sandbox(struct ksm *k, pid_t pid);

ksm/ksm/ksm.vcxproj

@@ -21,7 +21,8 @@
   </PropertyGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
-    <TargetVersion>Windows7</TargetVersion>
+    <TargetVersion>
+    </TargetVersion>
     <UseDebugLibraries>true</UseDebugLibraries>
     <PlatformToolset>WindowsKernelModeDriver10.0</PlatformToolset>
     <ConfigurationType>Driver</ConfigurationType>

list.h

@@ -338,7 +338,7 @@ static inline void list_splice_tail_init(struct list_head *list,
 * @member:	the name of the list_head within the struct.
 */
 #define list_entry(ptr, pos, member) \
-	((const char *)ptr - container_off_var(pos, member))
+	container_of_var(ptr, pos, member)
 
 /**
 * list_first_entry - get the first element from a list

print.c

@@ -158,7 +158,7 @@ static inline void print_flush(void)
 	smp_rmb();
 	KeAcquireInStackQueuedSpinLock(&lock, &q);
 	printbuf = buf + next_off + ((next & (PRINT_BUF_BUFFERS - 1)) << PRINT_BUF_SHIFT);
-	max = next_use - head_use - 1;
+	max = next_use - head_use;
 
 	strncpy(on_stack, printbuf, PRINT_BUF_ATONCE);
 	on_stack[PRINT_BUF_ATONCE] = '\0';

sandbox.c

@@ -35,7 +35,7 @@
  *	etc.), this is just a physical memory sandboxer.
  *
  *	This is basically CoW (copy-on-write) implementation but on the
- *	physical level, so when a registered application attempts to access a
+ *	physical level, so when a registered application writes to a
  *	memory region, another region is allocated and the original one is
  *	redirected to that one with a copy of the contents in it, then only
  *	that application will see that copy and not others.
@@ -90,7 +90,7 @@ static inline void free_sa_task(struct ksm *k, struct sa_task *task)
 
 	for (i = 0; i < KSM_MAX_VCPUS; ++i) {
 		if (task->eptp[i] != EPT_MAX_EPTP_LIST) {
-			ept = &ksm_cpu(k)->ept;
+			ept = &ksm_cpu_at(k, i)->ept;
 			ept_free_ptr(ept, task->eptp[i]);
 		}
 	}
@@ -234,7 +234,7 @@ int ksm_unbox(struct ksm *k, pid_t pid)
 			break;
 		}
 	}
-
+	spin_unlock(&k->task_lock);
 	return ret;
 }
 
@@ -243,19 +243,31 @@ static struct sa_task *find_sa_task_pgd(struct ksm *k, u64 pgd)
 	struct sa_task *task = NULL;
 	struct sa_task *ret = NULL;
 
-	spin_lock(&k->task_lock);
 	list_for_each_entry(task, &k->task_list, link) {
 		if (task->pgd == pgd) {
 			ret = task;
 			break;
 		}
 	}
-	spin_unlock(&k->task_lock);
+	return ret;
+}
+
+static struct sa_task *find_sa_task_pgd_pid(struct ksm *k, pid_t pid, u64 pgd)
+{
+	struct sa_task *task = NULL;
+	struct sa_task *ret = NULL;
+
+	list_for_each_entry(task, &k->task_list, link) {
+		if (task->pgd == pgd || task->pid == pid) {
+			ret = task;
+			break;
+		}
+	}
 	return ret;
 }
 
 bool ksm_sandbox_handle_ept(struct ept *ept, int dpl, u64 gpa,
-			    u64 gva, u16 curr, u8 ar, u8 ac,
+			    u64 gva, u64 cr3, u16 curr, u8 ar, u8 ac,
 			    bool *invd, u16 *eptp_switch)
 {
 	struct sa_task *task;
@@ -264,10 +276,13 @@ bool ksm_sandbox_handle_ept(struct ept *ept, int dpl, u64 gpa,
 	struct ksm *k;
 	u64 *epte;
 	u16 eptp;
+	pid_t pid;
 
 	vcpu = container_of(ept, struct vcpu, ept);
 	k = vcpu_to_ksm(vcpu);
-	task = find_sa_task(k, proc_pid());
+
+	pid = proc_id();
+	task = find_sa_task_pgd_pid(k, pid, cr3 & PAGE_PA_MASK);
 	if (!task) {
 		dbgbreak();
 		return false;
@@ -279,17 +294,14 @@ bool ksm_sandbox_handle_ept(struct ept *ept, int dpl, u64 gpa,
 	epte = ept_pte(EPT4(ept, curr), gpa);
 	BUG_ON(eptp != curr);
 
-	VCPU_DEBUG("%s: sandbox violation\n", proc_name());
 	if (dpl != 0 && ac & EPT_ACCESS_WRITE) {
-		VCPU_DEBUG("%s: allocating cow page\n", proc_name());
 		page = ksm_sandbox_copy_page(task, gpa);
 		if (!page)
 			return false;
 
 		__set_epte_ar_pfn(epte, ar | ac, page->hpa >> PAGE_SHIFT);
 		*invd = true;
 	} else {
-		VCPU_DEBUG("%s: let through\n", proc_name());
 		__set_epte_ar(epte, ar | ac);
 		*invd = true;
 	}

vcpu.c

@@ -294,20 +294,19 @@ u64 *ept_pte(u64 *pml4, u64 gpa)
 
 /*
  * Called from:
- *	- ept_handle_violation() aka VM Exit violation
- *	- __ept_handle_violation()
+ *	- ept_handle_violation() aka VMX root mode (host mode)
+ *	- __ept_handle_violation() aka IDT #VE (guest mode)
  *
- * Returns the EPTP index to be switched to if needed, or the current one
- * (eptp) if no switching is required,  If invalidation is required, @invd is
- * going to be true, do note that invalidation can only occur inside VMX root
+ * @eptp_switch is modified if switching is needed.
+ * If invalidation is required, @invd will be set,
+ * do note that invalidation can only occur inside VMX root
  * mode, and it's not required in non-root (#VE).
  *
- * If an error occurs, it returns EPT_MAX_EPTP_LIST which is 512.
  * Note that we don't need to invalidate non existent entries, aka entries that
  * mostly have EPT_ACCESS_NONE which is usually not even allocated...
  */
 static bool do_ept_violation(struct vcpu *vcpu, u64 rip, int dpl, u64 gpa,
-			     u64 gva, u16 eptp, u8 ar, u8 ac,
+			     u64 gva, u64 cr3, u16 eptp, u8 ar, u8 ac,
 			     bool *invd, u16 *eptp_switch)
 {
 	struct ept *ept = &vcpu->ept;
@@ -335,10 +334,10 @@ static bool do_ept_violation(struct vcpu *vcpu, u64 rip, int dpl, u64 gpa,
 
 #ifdef PMEM_SANDBOX
 	if (ksm_sandbox_handle_ept(&vcpu->ept, dpl, gpa,
-				   gva, eptp, ar, ac,
+				   gva, cr3, eptp, ar, ac,
 				   invd, eptp_switch)) {
 		if (*eptp_switch != eptp)
-			VCPU_DEBUG("sandbox switch from %d  to %d\n", eptp, *eptp_switch);
+			VCPU_DEBUG("sandbox switch from %d to %d\n", eptp, *eptp_switch);
 
 		return true;
 	}
@@ -353,7 +352,7 @@ static bool do_ept_violation(struct vcpu *vcpu, u64 rip, int dpl, u64 gpa,
  */
 bool ept_handle_violation(struct vcpu *vcpu)
 {
-	u64 exit, gpa, gva;
+	u64 exit, gpa, gva, cr3;
 	u16 eptp, eptp_switch;
 	u8 ar, ac;
 	char sar[4], sac[4];
@@ -362,6 +361,7 @@ bool ept_handle_violation(struct vcpu *vcpu)
 
 	eptp = vcpu_eptp_idx(vcpu);
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+	cr3 = vmcs_read(GUEST_CR3);
 	dpl = VMX_AR_DPL(vmcs_read32(GUEST_SS_AR_BYTES));
 	gva = 0;
 	exit = vmcs_read(EXIT_QUALIFICATION);
@@ -377,7 +377,7 @@ bool ept_handle_violation(struct vcpu *vcpu)
 
 	eptp_switch = eptp;
 	if (!do_ept_violation(vcpu, vcpu->ip, dpl, gpa,
-			      gva, eptp, ar, ac,
+			      gva, cr3, eptp, ar, ac,
 			      &invd, &eptp_switch))
 		return false;
 
@@ -422,7 +422,7 @@ void __ept_handle_violation(uintptr_t cs, uintptr_t rip)
 
 	eptp_switch = eptp;
 	if (!do_ept_violation(vcpu, vcpu->ip, cs & 3, gpa,
-			      gva, eptp, ar, ac,
+			      gva, __readcr3(), eptp, ar, ac,
 			      &invd, &eptp_switch))
 		VCPU_BUGCHECK(EPT_BUGCHECK_CODE, EPT_UNHANDLED_VIOLATION, rip, gpa);