Re: [PATCH 19/21] i386 Kprobes semaphore fix

[Zachary Amsden <zach@vmware.com>]

Ingo Molnar wrote:

>could these bugs lead to local rootholes, if kprobes are enabled?
>
>The threat model we care about is: administrator has a few dozen kprobes 
>activated, which sample various aspects of the system. Unprivileged 
>userspace tries to exploit this fact: it has full control over its own 
>conduct (including the ability to create LDTs, vm86 mode and weird 
>segments), but unprivileged userspace has no ability to 
>activate/deactivate kprobes.
>
>could this cause problems?
>  
>

Good - it was thinking along these lines that led me to this patch.  It 
is still possible to defeat the EIP checking if you are crafty enough, 
because there is a window of opportunity from the time a fault is 
induced until the LDT is read where another CPU can come along and 
modify the LDT.  Note the GDT does not have this problem, as remote 
changes (via ptrace I think I saw code that makes it possible) will not 
take effect until the thread is rescheduled.  But with kernel 
preemption, even GDT based segments have the race.  Pardon my thinking 
out loud - this is a very sticky situation.

Actually, perhaps this window could be closed; we will have a pending 
LDT reload posted by IPI, or a reschedule (and thus TLS reload) pending, 
and we can hold processing of that even while enabling interrupts.  
There is a problem - the LDT will have already been modified, and we 
don't always re-allocate a new page for it, so the old data may be 
gone.  Seems like we could find ways to close the race, but it seems 
difficult to verify as correct, and may cost performance.

So my approach is to ignore the race; instead, I use limit checking.  
Note the limit here is specified in linear (non-segmented) virtual 
memory, and is clamped to MIN(limit,PAGE_OFFSET) for userspace probes.

+
+	/* Don't let userspace races re-address into kernel space */
+	if ((unsigned long)addr > limit)
+		return 0;
+

Also, added this test:

+		if (user_mode(regs))
+			__get_user(instr, (unsigned char __user *) addr);
+		else
+			instr = *addr;
+			


Which saves the kernel from faulting due to "impersonated non-linear 
breakpoints".  Note the OOPs should have been caught and safely dealt 
with anyway.

So there are still a couple of crafty tricks that can cause you to 
re-address into kernel range EIPs, but limit checking protects us, and 
in general, perhaps we need not worry about strict correctness of 
kprobes for these edge cases.  Now that I have the issue at attention, 
is it even possible / useful to set kprobes in a specific userspace 
context, or should only kernel breakpoints be considered for kprobes?  
Reading the code, it was not clear, and I could not find proper 
documentation, so I left the semantics as I saw them.

If indeed, as I suspect, kprobes are only for kernel code, then the code 
here can be greatly simplified by discarding user code segments right 
away and doing flat EIP conversion.

Zach