Chris Wright
2006-Feb-09 21:33 UTC
[Xen-devel] [RFC/PATCH] sanitize prvicmd hypercall args
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@sous-sol.org> --- 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@lists.xensource.com http://lists.xensource.com/xen-devel