From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felix Schmoll Subject: Re: [GSoC] GSoC Introduction : Fuzzing Xen hypercall interface Date: Mon, 20 Mar 2017 17:47:32 +0100 Message-ID: References: <2C3140B8-9B96-44F8-A4EA-CDBC07479379@gmail.com> <20170313111439.abjbrw5hyu4eda7y@citrix.com> <20170316162731.l4hzdjky34vsgjkc@citrix.com> <20170320161847.kic6b524lodgr25u@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1615317181167759706==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cq0T2-0004sI-RW for xen-devel@lists.xenproject.org; Mon, 20 Mar 2017 16:47:37 +0000 Received: by mail-it0-f66.google.com with SMTP id y18so18380125itc.2 for ; Mon, 20 Mar 2017 09:47:34 -0700 (PDT) In-Reply-To: <20170320161847.kic6b524lodgr25u@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Wei Liu Cc: xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============1615317181167759706== Content-Type: multipart/alternative; boundary=001a11446c98ab1d19054b2c486a --001a11446c98ab1d19054b2c486a Content-Type: text/plain; charset=UTF-8 2017-03-20 17:18 GMT+01:00 Wei Liu : > On Mon, Mar 20, 2017 at 09:12:54AM +0100, Felix Schmoll wrote: > > 2017-03-16 17:27 GMT+01:00 Wei Liu : > > > > #undef COMP > > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > > index 4b87c60845..de07ee529b 100644 > > --- a/xen/common/kernel.c > > +++ b/xen/common/kernel.c > > @@ -226,6 +226,12 @@ void __init do_initcalls(void) > > * Simple hypercalls. > > */ > > > > +DO(domain_id)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > XEN_GUEST_HANDLE_PARAM means arg is a pointer to void type inside of the > guest, it's better to just use XEN_GUEST_HANDLE_PARAM(uint32_t) arg. > Also see below. > > > +{ > > + struct domain *d = current->domain; > > + return d->domain_id; > > You certainly don't need cmd, because you provide no command. > > If you want to return the domain id directly, you don't need arg. > Also please be aware of the types (long vs uint32_t). I think this is > the simplest approach. > > > +} > > + > > DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > { > > bool_t deny = !!xsm_xen_version(XSM_OTHER, cmd); > > diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > > index 91ba8bb48e..4ad62aa01b 100644 > > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -121,6 +121,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); > > #define __HYPERVISOR_xc_reserved_op 39 /* reserved for XenClient > */ > > #define __HYPERVISOR_xenpmu_op 40 > > #define __HYPERVISOR_dm_op 41 > > +#define __HYPERVISOR_domain_id 42 /* custom hypercall */ > > > > /* Architecture-specific hypercall definitions. */ > > #define __HYPERVISOR_arch_0 48 > > diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h > > index cc99aea57d..438684df85 100644 > > --- a/xen/include/xen/hypercall.h > > +++ b/xen/include/xen/hypercall.h > > @@ -83,6 +83,11 @@ do_xen_version( > > XEN_GUEST_HANDLE_PARAM(void) arg); > > > > extern long > > +do_domain_id( > > + int cmd, > > + XEN_GUEST_HANDLE_PARAM(void) arg); > > + > > +extern long > > do_console_io( > > int cmd, > > int count, > > -- > > 2.11.0 > > > > > > From 3a896fcf3bc5d7f8c4613d9d4b854684ec981e7f Mon Sep 17 00:00:00 2001 > > From: Felix Schmoll > > Date: Thu, 16 Mar 2017 22:38:23 +0100 > > Subject: [PATCH 2/2] Adjust libxc to new hypercall (untested) > > > > --- > > tools/libxc/include/xenctrl.h | 1 + > > tools/libxc/xc_private.c | 17 +++++++++++++++++ > > tools/libxc/xc_private.h | 8 ++++++++ > > 3 files changed, 26 insertions(+) > > > > diff --git a/tools/libxc/include/xenctrl.h > b/tools/libxc/include/xenctrl.h > > index a48981abea..e454b10f64 100644 > > --- a/tools/libxc/include/xenctrl.h > > +++ b/tools/libxc/include/xenctrl.h > > @@ -1546,6 +1546,7 @@ int xc_domctl(xc_interface *xch, struct xen_domctl > > *domctl); > > int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl); > > > > int xc_version(xc_interface *xch, int cmd, void *arg); > > +int xc_domid(xc_interface *xch, int cmd, void *arg); > > > > int xc_flask_op(xc_interface *xch, xen_flask_op_t *op); > > > > diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c > > index 72e6242417..f4f158c661 100644 > > --- a/tools/libxc/xc_private.c > > +++ b/tools/libxc/xc_private.c > > @@ -530,6 +530,23 @@ int xc_version(xc_interface *xch, int cmd, void > *arg) > > return rc; > > } > > > > +int xc_domid(xc_interface *xch, int cmd, void *arg) > > +{ > > + DECLARE_HYPERCALL_BOUNCE(arg, 0, XC_HYPERCALL_BUFFER_BOUNCE_OUT); > /* > > Size unknown until cmd decoded */ > > + size_t sz; > > + int rc; > > + > > + /* TODO: might want to either consider or remove cmd param */ > > + sz = 0; > > + > > + HYPERCALL_BOUNCE_SET_SIZE(arg, sz); > > + > > + rc = do_domain_id(xch, cmd, HYPERCALL_BUFFER(arg)); > > Most likely you will get rc = 0 because you're running your test program > in Dom0. Try running this a DomU? > > > + > > + return rc; > > +} > > + > > + > > unsigned long xc_make_page_below_4G( > > xc_interface *xch, uint32_t domid, unsigned long mfn) > > { > > diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h > > index 1c27b0fded..7cf875ffb5 100644 > > --- a/tools/libxc/xc_private.h > > +++ b/tools/libxc/xc_private.h > > @@ -229,6 +229,14 @@ static inline int do_xen_version(xc_interface *xch, > > int cmd, xc_hypercall_buffer > > cmd, HYPERCALL_BUFFER_AS_ARG(dest)); > > } > > > > +/* custom hypercall */ > > +static inline int do_domain_id(xc_interface *xch, int cmd, > > xc_hypercall_buffer_t *dest) > > +{ > > + DECLARE_HYPERCALL_BUFFER_ARGUMENT(dest); > > + return xencall2(xch->xcall, __HYPERVISOR_domain_id, > > + cmd, HYPERCALL_BUFFER_AS_ARG(dest)); > > +} > > + > > static inline int do_physdev_op(xc_interface *xch, int cmd, void *op, > > size_t len) > > { > > int ret = -1; > > -- > > 2.11.0 > > > > Now the question is how do I debug the hypercall. I compared what I have > to > > xen_version and to the patch for the last implemented hypercall but you > > seem to have slightly changed the entry into to hypervisor since then. I > > could use gdb for debugging the libxc-part, and printk is fine for the > > kernel, but can you either point me to some discussion of the call-chain > or > > to some sort of debugger. > > > > You can't attach a debugger to hypervisor. You can use printk in > hypervisor to debug. > > Hypercall is just like syscall. You cross the boundary of different > privilege levels. You've already identified the C entry point > (x86/hypercall.c). For the ASM entry point, the PV one is in > arch/x86/x86_64/entry.S and the HVM one is in arch/x86/hvm/vmx/vmx.c > (Intel CPU) and arch/x86/svm/svm.c (AMD CPU). I don't think you need to > care about the ASM entry points though. > > Please always develop against xen.git staging branch. > > I don't follow why your code doesn't work for you. Can you be more > specific? > It returns -1 when I run the following program: #include #include int main(void) { xc_interface *xch = xc_interface_open(NULL, NULL, 0); int ver = xc_version(xch, XENVER_version, NULL); printf("Xen Version %d.%d\n", ver >> 16, ver & 0xffff); int domid = xc_domid(xch, 0, NULL); printf("Xen domain: %d\n", domid); xc_interface_close(xch); return 0; } Wei. > > > Any comments greatly appreciated. > > > > Felix > --001a11446c98ab1d19054b2c486a Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

= 2017-03-20 17:18 GMT+01:00 Wei Liu <wei.liu2@citrix.com>:<= br>
On Mon, Mar 20, 2017 at 09:12:54AM +0100, Felix Schmoll wrote:
> 2017-03-16 17:27 GMT+01:00 Wei Liu <wei.liu2@citrix.com>:
>
>=C2=A0 #undef COMP
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 4b87c60845..de07ee529b 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -226,6 +226,12 @@ void __init do_initcalls(void)
>=C2=A0 =C2=A0* Simple hypercalls.
>=C2=A0 =C2=A0*/
>
> +DO(domain_id)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)

XEN_GUEST_HANDLE_PARAM means arg is a pointer to void type inside of= the
guest, it's better to just use XEN_GUEST_HANDLE_PARAM(uint32_t) ar= g.
Also see below.

> +{
> +=C2=A0 =C2=A0 struct domain *d =3D current->domain;
> +=C2=A0 =C2=A0 return d->domain_id;

You certainly don't need cmd, because you provide no command.
If you want to return the domain id directly, you don't need arg.
Also please be aware of the types (long vs uint32_t). I think this is
the simplest approach.

> +}
> +
>=C2=A0 DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 bool_t deny =3D !!xsm_xen_version(XSM_OTHER, cmd);=
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 91ba8bb48e..4ad62aa01b 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -121,6 +121,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>=C2=A0 #define __HYPERVISOR_xc_reserved_op=C2=A0 =C2=A0 =C2=A0 =C2=A039= /* reserved for XenClient */
>=C2=A0 #define __HYPERVISOR_xenpmu_op=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 40
>=C2=A0 #define __HYPERVISOR_dm_op=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 41
> +#define __HYPERVISOR_domain_id=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 42 /* custom hypercall */
>
>=C2=A0 /* Architecture-specific hypercall definitions. */
>=C2=A0 #define __HYPERVISOR_arch_0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A048
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h=
> index cc99aea57d..438684df85 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -83,6 +83,11 @@ do_xen_version(
>=C2=A0 =C2=A0 =C2=A0 XEN_GUEST_HANDLE_PARAM(void) arg);
>
>=C2=A0 extern long
> +do_domain_id(
> +=C2=A0 =C2=A0 int cmd,
> +=C2=A0 =C2=A0 XEN_GUEST_HANDLE_PARAM(void) arg);
> +
> +extern long
>=C2=A0 do_console_io(
>=C2=A0 =C2=A0 =C2=A0 int cmd,
>=C2=A0 =C2=A0 =C2=A0 int count,
> --
> 2.11.0
>
>
> From 3a896fcf3bc5d7f8c4613d9d4b854684ec981e7f Mon Sep 17 00:00:00= 2001
> From: Felix Schmoll <= eggi.innovations@gmail.com>
> Date: Thu, 16 Mar 2017 22:38:23 +0100
> Subject: [PATCH 2/2] Adjust libxc to new hypercall (untested)
>
> ---
>=C2=A0 tools/libxc/include/xenctrl.h |=C2=A0 1 +
>=C2=A0 tools/libxc/xc_private.c=C2=A0 =C2=A0 =C2=A0 | 17 ++++++++++++++= +++
>=C2=A0 tools/libxc/xc_private.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 8 ++++++++<= br> >=C2=A0 3 files changed, 26 insertions(+)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/= xenctrl.h
> index a48981abea..e454b10f64 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1546,6 +1546,7 @@ int xc_domctl(xc_interface *xch, struct xen_domc= tl
> *domctl);
>=C2=A0 int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl);
>
>=C2=A0 int xc_version(xc_interface *xch, int cmd, void *arg);
> +int xc_domid(xc_interface *xch, int cmd, void *arg);
>
>=C2=A0 int xc_flask_op(xc_interface *xch, xen_flask_op_t *op);
>
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index 72e6242417..f4f158c661 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -530,6 +530,23 @@ int xc_version(xc_interface *xch, int cmd, void *= arg)
>=C2=A0 =C2=A0 =C2=A0 return rc;
>=C2=A0 }
>
> +int xc_domid(xc_interface *xch, int cmd, void *arg)
> +{
> +=C2=A0 =C2=A0 DECLARE_HYPERCALL_BOUNCE(arg, 0, XC_HYPERCALL_BUFFER_BO= UNCE_OUT); /*
> Size unknown until cmd decoded */
> +=C2=A0 =C2=A0 size_t sz;
> +=C2=A0 =C2=A0 int rc;
> +
> +=C2=A0 =C2=A0 /* TODO: might want to either consider or remove cmd pa= ram */
> +=C2=A0 =C2=A0 sz =3D 0;
> +
> +=C2=A0 =C2=A0 HYPERCALL_BOUNCE_SET_SIZE(arg, sz);
> +
> +=C2=A0 =C2=A0 rc =3D do_domain_id(xch, cmd, HYPERCALL_BUFFER(arg));
Most likely you will get rc =3D 0 because you're running yo= ur test program
in Dom0. Try running this a DomU?

> +
> +=C2=A0 =C2=A0 return rc;
> +}
> +
> +
>=C2=A0 unsigned long xc_make_page_below_4G(
>=C2=A0 =C2=A0 =C2=A0 xc_interface *xch, uint32_t domid, unsigned long m= fn)
>=C2=A0 {
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 1c27b0fded..7cf875ffb5 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -229,6 +229,14 @@ static inline int do_xen_version(xc_interface *xc= h,
> int cmd, xc_hypercall_buffer
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 cmd, HYPERCALL_BUFFER_AS_ARG(dest));
>=C2=A0 }
>
> +/* custom hypercall */
> +static inline int do_domain_id(xc_interface *xch, int cmd,
> xc_hypercall_buffer_t *dest)
> +{
> +=C2=A0 =C2=A0 DECLARE_HYPERCALL_BUFFER_ARGUMENT(dest);
> +=C2=A0 =C2=A0 return xencall2(xch->xcall, __HYPERVISOR_domain_id,<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= cmd, HYPERCALL_BUFFER_AS_ARG(dest));
> +}
> +
>=C2=A0 static inline int do_physdev_op(xc_interface *xch, int cmd, void= *op,
> size_t len)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 int ret =3D -1;
> --
> 2.11.0
>
> Now the question is how do I debug the hypercall. I compared what I ha= ve to
> xen_version and to the patch for the last implemented hypercall but yo= u
> seem to have slightly changed the entry into to hypervisor since then.= I
> could use gdb for debugging the libxc-part, and printk is fine for the=
> kernel, but can you either point me to some discussion of the call-cha= in or
> to some sort of debugger.
>

You can't attach a debugger to hypervisor. You can use prin= tk in
hypervisor to debug.

Hypercall is just like syscall. You cross the boundary of different
privilege levels. You've already identified the C entry point
(x86/hypercall.c). For the ASM entry point, the PV one is in
arch/x86/x86_64/entry.S and the HVM one is in arch/x86/hvm/vmx/vmx.c
(Intel CPU) and arch/x86/svm/svm.c (AMD CPU). I don't think you need to=
care about the ASM entry points though.

Please always develop against xen.git staging branch.

I don't follow why your code doesn't work for you. Can you be more<= br> specific?

It returns -1 when I run the = following program:

#include <stdio.h>
#include <xenctrl.h>

int main(void) = {
=C2=A0 =C2=A0 xc_interface *xch =3D xc_interface_open(NULL, NUL= L, 0);

=C2=A0 =C2=A0 int ver =3D xc_version(xch, X= ENVER_version, NULL);

=C2=A0 =C2=A0 printf("X= en Version %d.%d\n", ver >> 16, ver & 0xffff);
=C2=A0 =C2=A0 int domid =3D xc_domid(xch, 0, NULL);
<= br>
=C2=A0 =C2=A0 printf("Xen domain: %d\n", domid);

=C2=A0 =C2=A0 xc_interface_close(xch);
=C2=A0 =C2=A0 return 0;
}

Wei.

> Any comments greatly appreciated.
>
> Felix

--001a11446c98ab1d19054b2c486a-- --===============1615317181167759706== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============1615317181167759706==--