The privcmd ioctl hypercall interface is unsafe. Luckily it's root
only, but it's a stability concern. The args are free form, passed
directly to the kernel, which will do smth akin to:
"shll $5,%%eax ;"
"addl $hypercall_page,%%eax ;"
"call *%%eax ;"
So this allows a call to anywhere in kernel space with arbitrary user values
in registers. Assuming it's a valid hypercall op, the args are passed
directly through. This facilitates easy way to scribble to random
kernel data from userspace. Again, as root only, this is less security
concern, and more stability issue. It would be better to provide
typesafe, and sanitized entrypoints for each hypercall. Short of that,
here's the best for discussion starters.
Signed-off-by: Chris Wright <chrisw@xxxxxxxxxxxx>
---
diff -r 10d6c1dc1bc7 linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c
--- a/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Thu Feb 9
12:17:35 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Thu Feb 9
15:55:47 2006 -0500
@@ -47,6 +47,12 @@ static int privcmd_ioctl(struct inode *i
if (copy_from_user(&hypercall, udata, sizeof(hypercall)))
return -EFAULT;
+
+ /* sanitize hypercall args, only safe one is .op
+ * although the whole argset should be sanitized
+ */
+ if (hypercall.op > NR_HYPERCALL_MAX)
+ return -EINVAL;
#if defined(__i386__)
__asm__ __volatile__ (
diff -r 10d6c1dc1bc7 xen/include/public/xen.h
--- a/xen/include/public/xen.h Thu Feb 9 12:17:35 2006 +0100
+++ b/xen/include/public/xen.h Thu Feb 9 15:55:47 2006 -0500
@@ -62,6 +62,8 @@
#define __HYPERVISOR_acm_op 27
#define __HYPERVISOR_nmi_op 28
+#define NR_HYPERCALL_MAX 28
+
/*
* VIRTUAL INTERRUPTS
*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home