All of lore.kernel.org
 help / color / mirror / Atom feed
* xc_evtchn_status fails with EFAULT on HVM, the same on PV works
@ 2017-01-13 17:31 Marek Marczykowski-Górecki
  2017-01-13 17:38 ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 17:31 UTC (permalink / raw)
  To: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 521 bytes --]

Hi,

I have a strange problem - xc_evtchn_status fails when running in HVM,
while exactly the same code (same kernel, same application etc) works
fine in PV. I've narrowed it down to copy_from_guest call in
common/event_channel.c, but no idea why it fails there. Xen version is
4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 17:31 xc_evtchn_status fails with EFAULT on HVM, the same on PV works Marek Marczykowski-Górecki
@ 2017-01-13 17:38 ` Andrew Cooper
  2017-01-13 18:03   ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-13 17:38 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki, xen-devel

On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> Hi,
>
> I have a strange problem - xc_evtchn_status fails when running in HVM,
> while exactly the same code (same kernel, same application etc) works
> fine in PV. I've narrowed it down to copy_from_guest call in
> common/event_channel.c, but no idea why it fails there. Xen version is
> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?

Which specific copy_from_guest() call?

Copying data out of a PV guest is different to copying out of a HVM
guest, but copy_from_guest() should cope properly with both.

However, to progress, it would help to know exactly which piece of data
is being requested.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 17:38 ` Andrew Cooper
@ 2017-01-13 18:03   ` Marek Marczykowski-Górecki
  2017-01-13 18:15     ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 18:03 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1545 bytes --]

On Fri, Jan 13, 2017 at 05:38:42PM +0000, Andrew Cooper wrote:
> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> > Hi,
> >
> > I have a strange problem - xc_evtchn_status fails when running in HVM,
> > while exactly the same code (same kernel, same application etc) works
> > fine in PV. I've narrowed it down to copy_from_guest call in
> > common/event_channel.c, but no idea why it fails there. Xen version is
> > 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
> 
> Which specific copy_from_guest() call?
> 
> Copying data out of a PV guest is different to copying out of a HVM
> guest, but copy_from_guest() should cope properly with both.
> 
> However, to progress, it would help to know exactly which piece of data
> is being requested.

This one:
https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104

    case EVTCHNOP_status: {
        struct evtchn_status status;
        if ( copy_from_guest(&status, arg, 1) != 0 )
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            return -EFAULT;
        rc = evtchn_status(&status);
        if ( !rc && __copy_to_guest(arg, &status, 1) )
            rc = -EFAULT;
        break;

The evtchn_status structure in application is on stack (local variable),
but I think it shouldn't matter, as libxc copy it to a bounce buffer.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 18:03   ` Marek Marczykowski-Górecki
@ 2017-01-13 18:15     ` Andrew Cooper
  2017-01-13 18:32       ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-13 18:15 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel

On 13/01/17 18:03, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 13, 2017 at 05:38:42PM +0000, Andrew Cooper wrote:
>> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
>>> Hi,
>>>
>>> I have a strange problem - xc_evtchn_status fails when running in HVM,
>>> while exactly the same code (same kernel, same application etc) works
>>> fine in PV. I've narrowed it down to copy_from_guest call in
>>> common/event_channel.c, but no idea why it fails there. Xen version is
>>> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
>> Which specific copy_from_guest() call?
>>
>> Copying data out of a PV guest is different to copying out of a HVM
>> guest, but copy_from_guest() should cope properly with both.
>>
>> However, to progress, it would help to know exactly which piece of data
>> is being requested.
> This one:
> https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104
>
>     case EVTCHNOP_status: {
>         struct evtchn_status status;
>         if ( copy_from_guest(&status, arg, 1) != 0 )
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>             return -EFAULT;
>         rc = evtchn_status(&status);
>         if ( !rc && __copy_to_guest(arg, &status, 1) )
>             rc = -EFAULT;
>         break;
>
> The evtchn_status structure in application is on stack (local variable),
> but I think it shouldn't matter, as libxc copy it to a bounce buffer.
>

The intent of bounce buffers is certainly to avoid this problem from
happening.

Is this a 32bit HVM guest?  Compat argument translation does make the
logic a little more complicated.

Can you get the result of this piece of debugging in the failure case?

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 638dc5e..ab5d82a 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1101,8 +1101,13 @@ long do_event_channel_op(int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case EVTCHNOP_status: {
         struct evtchn_status status;
-        if ( copy_from_guest(&status, arg, 1) != 0 )
+        unsigned int res = copy_from_guest(&status, arg, 1);
+        if ( res != 0 )
+        {
+            printk("** %pv CFG(%zu, %p, 1) = %u\n",
+                   current, sizeof(status), _p(arg.p), res);
             return -EFAULT;
+        }
         rc = evtchn_status(&status);
         if ( !rc && __copy_to_guest(arg, &status, 1) )
             rc = -EFAULT;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 18:15     ` Andrew Cooper
@ 2017-01-13 18:32       ` Marek Marczykowski-Górecki
  2017-01-13 18:37         ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 18:32 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 2942 bytes --]

On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
> On 13/01/17 18:03, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 05:38:42PM +0000, Andrew Cooper wrote:
> >> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> >>> Hi,
> >>>
> >>> I have a strange problem - xc_evtchn_status fails when running in HVM,
> >>> while exactly the same code (same kernel, same application etc) works
> >>> fine in PV. I've narrowed it down to copy_from_guest call in
> >>> common/event_channel.c, but no idea why it fails there. Xen version is
> >>> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
> >> Which specific copy_from_guest() call?
> >>
> >> Copying data out of a PV guest is different to copying out of a HVM
> >> guest, but copy_from_guest() should cope properly with both.
> >>
> >> However, to progress, it would help to know exactly which piece of data
> >> is being requested.
> > This one:
> > https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104
> >
> >     case EVTCHNOP_status: {
> >         struct evtchn_status status;
> >         if ( copy_from_guest(&status, arg, 1) != 0 )
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >             return -EFAULT;
> >         rc = evtchn_status(&status);
> >         if ( !rc && __copy_to_guest(arg, &status, 1) )
> >             rc = -EFAULT;
> >         break;
> >
> > The evtchn_status structure in application is on stack (local variable),
> > but I think it shouldn't matter, as libxc copy it to a bounce buffer.
> >
> 
> The intent of bounce buffers is certainly to avoid this problem from
> happening.
> 
> Is this a 32bit HVM guest?  Compat argument translation does make the
> logic a little more complicated.

No, its 64bit guest.

> Can you get the result of this piece of debugging in the failure case?

I've got this:
** d4v0 CFG(24, 00007f794bd07004, 1) = 24

> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 638dc5e..ab5d82a 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -1101,8 +1101,13 @@ long do_event_channel_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case EVTCHNOP_status: {
>          struct evtchn_status status;
> -        if ( copy_from_guest(&status, arg, 1) != 0 )
> +        unsigned int res = copy_from_guest(&status, arg, 1);
> +        if ( res != 0 )
> +        {
> +            printk("** %pv CFG(%zu, %p, 1) = %u\n",
> +                   current, sizeof(status), _p(arg.p), res);
>              return -EFAULT;
> +        }
>          rc = evtchn_status(&status);
>          if ( !rc && __copy_to_guest(arg, &status, 1) )
>              rc = -EFAULT;

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 18:32       ` Marek Marczykowski-Górecki
@ 2017-01-13 18:37         ` Andrew Cooper
  2017-01-13 18:59           ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-13 18:37 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel

On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
>> On 13/01/17 18:03, Marek Marczykowski-Górecki wrote:
>>> On Fri, Jan 13, 2017 at 05:38:42PM +0000, Andrew Cooper wrote:
>>>> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
>>>>> Hi,
>>>>>
>>>>> I have a strange problem - xc_evtchn_status fails when running in HVM,
>>>>> while exactly the same code (same kernel, same application etc) works
>>>>> fine in PV. I've narrowed it down to copy_from_guest call in
>>>>> common/event_channel.c, but no idea why it fails there. Xen version is
>>>>> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
>>>> Which specific copy_from_guest() call?
>>>>
>>>> Copying data out of a PV guest is different to copying out of a HVM
>>>> guest, but copy_from_guest() should cope properly with both.
>>>>
>>>> However, to progress, it would help to know exactly which piece of data
>>>> is being requested.
>>> This one:
>>> https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104
>>>
>>>     case EVTCHNOP_status: {
>>>         struct evtchn_status status;
>>>         if ( copy_from_guest(&status, arg, 1) != 0 )
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>             return -EFAULT;
>>>         rc = evtchn_status(&status);
>>>         if ( !rc && __copy_to_guest(arg, &status, 1) )
>>>             rc = -EFAULT;
>>>         break;
>>>
>>> The evtchn_status structure in application is on stack (local variable),
>>> but I think it shouldn't matter, as libxc copy it to a bounce buffer.
>>>
>> The intent of bounce buffers is certainly to avoid this problem from
>> happening.
>>
>> Is this a 32bit HVM guest?  Compat argument translation does make the
>> logic a little more complicated.
> No, its 64bit guest.
>
>> Can you get the result of this piece of debugging in the failure case?
> I've got this:
> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24

Silly question (and I really hope the answer isn't yes, but I have a
sinking feeling it is).

Is the guest in question using SMAP? If so, does disabling SMAP fix the
problem?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 18:37         ` Andrew Cooper
@ 2017-01-13 18:59           ` Marek Marczykowski-Górecki
  2017-01-13 19:27             ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 18:59 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 986 bytes --]

On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
> >> Can you get the result of this piece of debugging in the failure case?
> > I've got this:
> > ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
> 
> Silly question (and I really hope the answer isn't yes, but I have a
> sinking feeling it is).
> 
> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> problem?

How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
v` output enough, then yes - it's enabled. And booting hypervisor with
smap=0 "fixed" the problem.
So, what would be the correct solution? I'd prefer not to disable SMAP
"just" for this reason...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 18:59           ` Marek Marczykowski-Górecki
@ 2017-01-13 19:27             ` Andrew Cooper
  2017-01-13 19:40               ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-13 19:27 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel

On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
>> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
>>> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
>>>> Can you get the result of this piece of debugging in the failure case?
>>> I've got this:
>>> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
>> Silly question (and I really hope the answer isn't yes, but I have a
>> sinking feeling it is).
>>
>> Is the guest in question using SMAP? If so, does disabling SMAP fix the
>> problem?
> How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
> v` output enough, then yes - it's enabled. And booting hypervisor with
> smap=0 "fixed" the problem.

:(, although now I think about it, there might be a quick option.

> So, what would be the correct solution? I'd prefer not to disable SMAP
> "just" for this reason...

For the quick option, the privcmd driver in Linux needs a stac()/clac()
pair around the actual hypercall invocation, to whitelist userspace
memory accesses as being ok.

Something like this (completely untested)

andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
diff --git a/arch/x86/include/asm/xen/hypercall.h
b/arch/x86/include/asm/xen/hypercall.h
index a12a047..e1b2af9e 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -214,10 +214,12 @@ privcmd_call(unsigned call,
        __HYPERCALL_DECLS;
        __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
 
+       stac();
        asm volatile("call *%[call]"
                     : __HYPERCALL_5PARAM
                     : [call] "a" (&hypercall_page[call])
                     : __HYPERCALL_CLOBBER5);
+       clac();
 
        return (long)__res;
 }

For the longer option, introducing a non-virtual ABI for Xen.  This is
going to become a necessary prerequisite to support AMD's Secure Virtual
Encryption technology (where the hypervisor deliberately cannot follow
the pagetables), and would remove the overhead of Xen having to walk the
guest pagetables.

Another optimisation would be to alter some of the ops to pass their
parameters in registers rather than in memory.  There are quite a few
ops which pass pointers to a single int, which could be completed more
efficiently by Xen (for both PV and HVM guests) by avoiding the memory
access entirely.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 19:27             ` Andrew Cooper
@ 2017-01-13 19:40               ` Marek Marczykowski-Górecki
  2017-01-13 19:54                 ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 19:40 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 3071 bytes --]

On Fri, Jan 13, 2017 at 07:27:20PM +0000, Andrew Cooper wrote:
> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
> >> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> >>> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
> >>>> Can you get the result of this piece of debugging in the failure case?
> >>> I've got this:
> >>> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
> >> Silly question (and I really hope the answer isn't yes, but I have a
> >> sinking feeling it is).
> >>
> >> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> >> problem?
> > How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
> > v` output enough, then yes - it's enabled. And booting hypervisor with
> > smap=0 "fixed" the problem.
> 
> :(, although now I think about it, there might be a quick option.
> 
> > So, what would be the correct solution? I'd prefer not to disable SMAP
> > "just" for this reason...
> 
> For the quick option, the privcmd driver in Linux needs a stac()/clac()
> pair around the actual hypercall invocation, to whitelist userspace
> memory accesses as being ok.
> 
> Something like this (completely untested)
> 
> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
> diff --git a/arch/x86/include/asm/xen/hypercall.h
> b/arch/x86/include/asm/xen/hypercall.h
> index a12a047..e1b2af9e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
>         __HYPERCALL_DECLS;
>         __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
>  
> +       stac();
>         asm volatile("call *%[call]"
>                      : __HYPERCALL_5PARAM
>                      : [call] "a" (&hypercall_page[call])
>                      : __HYPERCALL_CLOBBER5);
> +       clac();
>  
>         return (long)__res;
>  }

Is there any option to do that from hypervisor side? For example somehow
modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
kernel (for example sometimes I need to cope with the one from Debian
stable).

> For the longer option, introducing a non-virtual ABI for Xen.  This is
> going to become a necessary prerequisite to support AMD's Secure Virtual
> Encryption technology (where the hypervisor deliberately cannot follow
> the pagetables), and would remove the overhead of Xen having to walk the
> guest pagetables.
> 
> Another optimisation would be to alter some of the ops to pass their
> parameters in registers rather than in memory.  There are quite a few
> ops which pass pointers to a single int, which could be completed more
> efficiently by Xen (for both PV and HVM guests) by avoiding the memory
> access entirely.

Yes, but it will not solve all the cases.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 19:40               ` Marek Marczykowski-Górecki
@ 2017-01-13 19:54                 ` Andrew Cooper
  2017-01-13 20:32                   ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-13 19:54 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel

On 13/01/17 19:40, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 13, 2017 at 07:27:20PM +0000, Andrew Cooper wrote:
>> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
>>> On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
>>>> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
>>>>> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
>>>>>> Can you get the result of this piece of debugging in the failure case?
>>>>> I've got this:
>>>>> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
>>>> Silly question (and I really hope the answer isn't yes, but I have a
>>>> sinking feeling it is).
>>>>
>>>> Is the guest in question using SMAP? If so, does disabling SMAP fix the
>>>> problem?
>>> How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
>>> v` output enough, then yes - it's enabled. And booting hypervisor with
>>> smap=0 "fixed" the problem.
>> :(, although now I think about it, there might be a quick option.
>>
>>> So, what would be the correct solution? I'd prefer not to disable SMAP
>>> "just" for this reason...
>> For the quick option, the privcmd driver in Linux needs a stac()/clac()
>> pair around the actual hypercall invocation, to whitelist userspace
>> memory accesses as being ok.
>>
>> Something like this (completely untested)
>>
>> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
>> diff --git a/arch/x86/include/asm/xen/hypercall.h
>> b/arch/x86/include/asm/xen/hypercall.h
>> index a12a047..e1b2af9e 100644
>> --- a/arch/x86/include/asm/xen/hypercall.h
>> +++ b/arch/x86/include/asm/xen/hypercall.h
>> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
>>         __HYPERCALL_DECLS;
>>         __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
>>  
>> +       stac();
>>         asm volatile("call *%[call]"
>>                      : __HYPERCALL_5PARAM
>>                      : [call] "a" (&hypercall_page[call])
>>                      : __HYPERCALL_CLOBBER5);
>> +       clac();
>>  
>>         return (long)__res;
>>  }
> Is there any option to do that from hypervisor side? For example somehow
> modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
> kernel (for example sometimes I need to cope with the one from Debian
> stable).

Not really.  (I mean - there is, but it involves deliberately breaking
SMAP support in Xen.)

This is a guest kernel bug.  The guest kernel has used SMAP to say
"please disallow all userspace accesses", and Xen is respecting this
when trying to follow the pointer in the hypercall.  This bug doesn't
manifest for PV guests, and you are probably the first person to do any
serious hypercall work from HVM userspace.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 19:54                 ` Andrew Cooper
@ 2017-01-13 20:32                   ` Marek Marczykowski-Górecki
  2017-01-14  1:47                     ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-13 20:32 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 3681 bytes --]

On Fri, Jan 13, 2017 at 07:54:01PM +0000, Andrew Cooper wrote:
> On 13/01/17 19:40, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 07:27:20PM +0000, Andrew Cooper wrote:
> >> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
> >>> On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
> >>>> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> >>>>> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
> >>>>>> Can you get the result of this piece of debugging in the failure case?
> >>>>> I've got this:
> >>>>> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
> >>>> Silly question (and I really hope the answer isn't yes, but I have a
> >>>> sinking feeling it is).
> >>>>
> >>>> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> >>>> problem?
> >>> How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
> >>> v` output enough, then yes - it's enabled. And booting hypervisor with
> >>> smap=0 "fixed" the problem.
> >> :(, although now I think about it, there might be a quick option.
> >>
> >>> So, what would be the correct solution? I'd prefer not to disable SMAP
> >>> "just" for this reason...
> >> For the quick option, the privcmd driver in Linux needs a stac()/clac()
> >> pair around the actual hypercall invocation, to whitelist userspace
> >> memory accesses as being ok.
> >>
> >> Something like this (completely untested)
> >>
> >> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
> >> diff --git a/arch/x86/include/asm/xen/hypercall.h
> >> b/arch/x86/include/asm/xen/hypercall.h
> >> index a12a047..e1b2af9e 100644
> >> --- a/arch/x86/include/asm/xen/hypercall.h
> >> +++ b/arch/x86/include/asm/xen/hypercall.h
> >> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
> >>         __HYPERCALL_DECLS;
> >>         __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
> >>  
> >> +       stac();
> >>         asm volatile("call *%[call]"
> >>                      : __HYPERCALL_5PARAM
> >>                      : [call] "a" (&hypercall_page[call])
> >>                      : __HYPERCALL_CLOBBER5);
> >> +       clac();
> >>  
> >>         return (long)__res;
> >>  }
> > Is there any option to do that from hypervisor side? For example somehow
> > modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
> > kernel (for example sometimes I need to cope with the one from Debian
> > stable).
> 
> Not really.  (I mean - there is, but it involves deliberately breaking
> SMAP support in Xen.)
> 
> This is a guest kernel bug.  The guest kernel has used SMAP to say
> "please disallow all userspace accesses", and Xen is respecting this
> when trying to follow the pointer in the hypercall.

Hmm, if that's the case, isn't the above patch also effectively breaking
SMAP? I see the purpose of SMAP is to prevent _accidental_ access to
arbitrary, guest controlled memory. For example because of some memory
corruption bug in the hypervisor. If such a bug could be triggered using
a hypercall, then the above patch also makes SMAP useless.  Patching
copy_from_guest/copy_to_guest on the other hand would only allow such
guest controlled memory access when hypervisor is really supposed to
access guest memory.

> This bug doesn't
> manifest for PV guests, and you are probably the first person to do any
> serious hypercall work from HVM userspace.

That's interesting. I'm just trying to use slightly modified
libxenchan...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-13 20:32                   ` Marek Marczykowski-Górecki
@ 2017-01-14  1:47                     ` Andrew Cooper
  2017-01-14  2:52                       ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-14  1:47 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel

On 13/01/2017 20:32, Marek Marczykowski-Górecki wrote:
> On Fri, Jan 13, 2017 at 07:54:01PM +0000, Andrew Cooper wrote:
>> On 13/01/17 19:40, Marek Marczykowski-Górecki wrote:
>>> On Fri, Jan 13, 2017 at 07:27:20PM +0000, Andrew Cooper wrote:
>>>> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
>>>>> On Fri, Jan 13, 2017 at 06:37:06PM +0000, Andrew Cooper wrote:
>>>>>> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
>>>>>>> On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
>>>>>>>> Can you get the result of this piece of debugging in the failure case?
>>>>>>> I've got this:
>>>>>>> ** d4v0 CFG(24, 00007f794bd07004, 1) = 24
>>>>>> Silly question (and I really hope the answer isn't yes, but I have a
>>>>>> sinking feeling it is).
>>>>>>
>>>>>> Is the guest in question using SMAP? If so, does disabling SMAP fix the
>>>>>> problem?
>>>>> How can I check that? If looking at 0x200000 CR4 bit in `xl debug-keys
>>>>> v` output enough, then yes - it's enabled. And booting hypervisor with
>>>>> smap=0 "fixed" the problem.
>>>> :(, although now I think about it, there might be a quick option.
>>>>
>>>>> So, what would be the correct solution? I'd prefer not to disable SMAP
>>>>> "just" for this reason...
>>>> For the quick option, the privcmd driver in Linux needs a stac()/clac()
>>>> pair around the actual hypercall invocation, to whitelist userspace
>>>> memory accesses as being ok.
>>>>
>>>> Something like this (completely untested)
>>>>
>>>> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
>>>> diff --git a/arch/x86/include/asm/xen/hypercall.h
>>>> b/arch/x86/include/asm/xen/hypercall.h
>>>> index a12a047..e1b2af9e 100644
>>>> --- a/arch/x86/include/asm/xen/hypercall.h
>>>> +++ b/arch/x86/include/asm/xen/hypercall.h
>>>> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
>>>>         __HYPERCALL_DECLS;
>>>>         __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
>>>>  
>>>> +       stac();
>>>>         asm volatile("call *%[call]"
>>>>                      : __HYPERCALL_5PARAM
>>>>                      : [call] "a" (&hypercall_page[call])
>>>>                      : __HYPERCALL_CLOBBER5);
>>>> +       clac();
>>>>  
>>>>         return (long)__res;
>>>>  }
>>> Is there any option to do that from hypervisor side? For example somehow
>>> modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
>>> kernel (for example sometimes I need to cope with the one from Debian
>>> stable).
>> Not really.  (I mean - there is, but it involves deliberately breaking
>> SMAP support in Xen.)
>>
>> This is a guest kernel bug.  The guest kernel has used SMAP to say
>> "please disallow all userspace accesses", and Xen is respecting this
>> when trying to follow the pointer in the hypercall.
> Hmm, if that's the case, isn't the above patch also effectively breaking
> SMAP?

No.

> I see the purpose of SMAP is to prevent _accidental_ access to
> arbitrary, guest controlled memory.

This is correct.  The point of SMAP is to prevent accidental access to
user memory.

The ABI of the privcmd hypercall ioctl() is to pass values straight to
the hypervisor, and it is a fact that most of these hypercalls contain
pointers to hypercall buffers, which reside in userspace.

(There is a separate thread ongoing questioning whether the ABI is
appropriate, but irrespective of those results, this is how the ABI
currently behaves.  A concerned person might note that anyone with an
open privcmd fd can issue any hypercall, including the ones only
intended for kernels, such as set_trap_table, update_va_mapping and iret.)

Therefore, it is an expected consequence that a hypercall passed blindly
from a userspace agent contains userspace pointers which Xen must follow
to complete the hypercall successfully.

> For example because of some memory
> corruption bug in the hypervisor. If such a bug could be triggered using
> a hypercall, then the above patch also makes SMAP useless.  Patching
> copy_from_guest/copy_to_guest on the other hand would only allow such
> guest controlled memory access when hypervisor is really supposed to
> access guest memory.

I don't follow this argument.

In a native situation, SMAP raises #PF if hardware tries to follow a
user pointer outside of an area whitelisted by the kernel.  (The
copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)

The choice of supervisor vs user in a pagewalk is a single bit, and Xen
(for better or worse, but realistically as a consequence of SMAP being
~10 years younger than HVM guests) accesses pointers from hypercalls as
a supervisor entity when walking the pagetables.  The key point here is
that Xen must emulate the hardware pagewalker when trying to find the
data being pointed to.

If Xen has a bug causing spurious accesses to HVM guests, then all bets
are already off wrt memory corruption, because real hardware isn't used
to make the access.

As indicated before, we could deliberately break the Xen pagewalker to
ignore SMAP.  However, that would be identical to "pretend the guest
actually whitelisted this access".  This would fix your symptoms, but
open up a hole in the case that userspace somehow managed to trick Xen
into thinking a legitimate hypercall had been made, at which point Xen
would ignore the last level of protection that SMAP was supposed to offer.

>> This bug doesn't
>> manifest for PV guests, and you are probably the first person to do any
>> serious hypercall work from HVM userspace.
> That's interesting. I'm just trying to use slightly modified
> libxenchan...

And I believe that you are first person (in 6 years of being a part of
upstream Xen) who I have positively identified as having used
libxenchan.  (There have been a few comments along the line of "oh yeah
- there is that libxenchan thing which might be worth looking at".)

It does raise some questions though.  Linux has (what is supposed to be
a) perfectly good /dev/xen/evtchn, for interacting with event channels. 
Why is libxenchan open-coding the hypercalls using a lower level
interface?  Can it be modified to use the higher level interfaces?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-14  1:47                     ` Andrew Cooper
@ 2017-01-14  2:52                       ` Marek Marczykowski-Górecki
  2017-01-16 12:17                         ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-14  2:52 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 5832 bytes --]

On Sat, Jan 14, 2017 at 01:47:49AM +0000, Andrew Cooper wrote:
> On 13/01/2017 20:32, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 07:54:01PM +0000, Andrew Cooper wrote:
> >> This is a guest kernel bug.  The guest kernel has used SMAP to say
> >> "please disallow all userspace accesses", and Xen is respecting this
> >> when trying to follow the pointer in the hypercall.
> > Hmm, if that's the case, isn't the above patch also effectively breaking
> > SMAP?
> 
> No.
> 
> > I see the purpose of SMAP is to prevent _accidental_ access to
> > arbitrary, guest controlled memory.
> 
> This is correct.  The point of SMAP is to prevent accidental access to
> user memory.
> 
> The ABI of the privcmd hypercall ioctl() is to pass values straight to
> the hypervisor, and it is a fact that most of these hypercalls contain
> pointers to hypercall buffers, which reside in userspace.
> 
> (There is a separate thread ongoing questioning whether the ABI is
> appropriate, but irrespective of those results, this is how the ABI
> currently behaves.  A concerned person might note that anyone with an
> open privcmd fd can issue any hypercall, including the ones only
> intended for kernels, such as set_trap_table, update_va_mapping and iret.)
> 
> Therefore, it is an expected consequence that a hypercall passed blindly
> from a userspace agent contains userspace pointers which Xen must follow
> to complete the hypercall successfully.
> 
> > For example because of some memory
> > corruption bug in the hypervisor. If such a bug could be triggered using
> > a hypercall, then the above patch also makes SMAP useless.  Patching
> > copy_from_guest/copy_to_guest on the other hand would only allow such
> > guest controlled memory access when hypervisor is really supposed to
> > access guest memory.
> 
> I don't follow this argument.
> 
> In a native situation, SMAP raises #PF if hardware tries to follow a
> user pointer outside of an area whitelisted by the kernel.  (The
> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
> 
> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
> (for better or worse, but realistically as a consequence of SMAP being
> ~10 years younger than HVM guests) accesses pointers from hypercalls as
> a supervisor entity when walking the pagetables.  The key point here is
> that Xen must emulate the hardware pagewalker when trying to find the
> data being pointed to.
> 
> If Xen has a bug causing spurious accesses to HVM guests, then all bets
> are already off wrt memory corruption, because real hardware isn't used
> to make the access.
> 
> As indicated before, we could deliberately break the Xen pagewalker to
> ignore SMAP.  However, that would be identical to "pretend the guest
> actually whitelisted this access". 

I think there is important difference between "whitelist all accesses
to guest user memory for the hypercall time" vs "whitelist accesses done
through copy_from_guest/copy_to_guest only". If I understand correctly
above description, modifying privcmd_call would also suppress #PF when
Xen would be tricked to access guest memory outside of copy_*_guest. Am
I missing something?

Anyway, if someone can access /dev/xen/privcmd and issue hypercalls,
probably also can whitelist userspace access... So the practical
difference is very small - apart from that I control Xen version, but
not necessary the kernel. And I think this applies to many more Xen
users (from which only I've tripped over this issue...). What is the
point of SMAP if guest can disable it? Is it only about protecting
hypervisor when attacker _does not_ control guest kernel?

> This would fix your symptoms, but
> open up a hole in the case that userspace somehow managed to trick Xen
> into thinking a legitimate hypercall had been made, at which point Xen
> would ignore the last level of protection that SMAP was supposed to offer.
> 
> >> This bug doesn't
> >> manifest for PV guests, and you are probably the first person to do any
> >> serious hypercall work from HVM userspace.
> > That's interesting. I'm just trying to use slightly modified
> > libxenchan...
> 
> And I believe that you are first person (in 6 years of being a part of
> upstream Xen) who I have positively identified as having used
> libxenchan.  (There have been a few comments along the line of "oh yeah
> - there is that libxenchan thing which might be worth looking at".)
> 
> It does raise some questions though.  Linux has (what is supposed to be
> a) perfectly good /dev/xen/evtchn, for interacting with event channels. 
> Why is libxenchan open-coding the hypercalls using a lower level
> interface?  Can it be modified to use the higher level interfaces?

Actually, vanilla libxenvchan do use /dev/xen/evtchn. I have a wrapper
library which additional check - if remote domain is still alive.
Otherwise there are multiple cases when libxenvchan will simply hang if
remote domain disappear without proper cleanup. Some of those cases are
covered by xengntshr_share_page_notify/xengnttab_map_grant_ref_notify
(namely: those cases when only application crashes, but kernel is still
alive). But not all...

The actual check use xc_evtchn_status() and observe result - if it get
unbound/interdomain status then the remote domain is still alive, but if
it get -ESRCH - then it's dead and application should consider the
connection broken.
Is there any better way to check if a domain with given ID is alive? Or
better - get a notification of its death (this all from domU)?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-14  2:52                       ` Marek Marczykowski-Górecki
@ 2017-01-16 12:17                         ` Jan Beulich
  2017-01-16 23:06                           ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2017-01-16 12:17 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: Andrew Cooper, xen-devel

>>> On 14.01.17 at 03:52, <marmarek@invisiblethingslab.com> wrote:
> On Sat, Jan 14, 2017 at 01:47:49AM +0000, Andrew Cooper wrote:
>> In a native situation, SMAP raises #PF if hardware tries to follow a
>> user pointer outside of an area whitelisted by the kernel.  (The
>> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
>> 
>> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
>> (for better or worse, but realistically as a consequence of SMAP being
>> ~10 years younger than HVM guests) accesses pointers from hypercalls as
>> a supervisor entity when walking the pagetables.  The key point here is
>> that Xen must emulate the hardware pagewalker when trying to find the
>> data being pointed to.
>> 
>> If Xen has a bug causing spurious accesses to HVM guests, then all bets
>> are already off wrt memory corruption, because real hardware isn't used
>> to make the access.
>> 
>> As indicated before, we could deliberately break the Xen pagewalker to
>> ignore SMAP.  However, that would be identical to "pretend the guest
>> actually whitelisted this access". 
> 
> I think there is important difference between "whitelist all accesses
> to guest user memory for the hypercall time" vs "whitelist accesses done
> through copy_from_guest/copy_to_guest only". If I understand correctly
> above description, modifying privcmd_call would also suppress #PF when
> Xen would be tricked to access guest memory outside of copy_*_guest. Am
> I missing something?

There are two additional things to consider here:

1) For HVM guests, Xen can't access guest memory unintentionally,
as (other than in the PV case) virtual address spaces are distinct.

2) When the guest issues stac()/clac(), it indicates to Xen _its own_
intended view, without affecting Xen's. That is, as soon as hypervisor
context is being entered again, SMAP protection would be in effect
again (albeit as per point 1 guarding only against accessing PV guest
mappings).

So the driver adjustment suggested by Andrew has an effect on only
page walks done by Xen during copy_{to,from}_guest(), but not on
actual memory accesses.

> Anyway, if someone can access /dev/xen/privcmd and issue hypercalls,
> probably also can whitelist userspace access... So the practical
> difference is very small - apart from that I control Xen version, but
> not necessary the kernel. And I think this applies to many more Xen
> users (from which only I've tripped over this issue...). What is the
> point of SMAP if guest can disable it? Is it only about protecting
> hypervisor when attacker _does not_ control guest kernel?

As per above - each entity controls its own security here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-16 12:17                         ` Jan Beulich
@ 2017-01-16 23:06                           ` Marek Marczykowski-Górecki
  2017-01-16 23:41                             ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-01-16 23:06 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 2697 bytes --]

On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
> >>> On 14.01.17 at 03:52, <marmarek@invisiblethingslab.com> wrote:
> > On Sat, Jan 14, 2017 at 01:47:49AM +0000, Andrew Cooper wrote:
> >> In a native situation, SMAP raises #PF if hardware tries to follow a
> >> user pointer outside of an area whitelisted by the kernel.  (The
> >> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
> >> 
> >> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
> >> (for better or worse, but realistically as a consequence of SMAP being
> >> ~10 years younger than HVM guests) accesses pointers from hypercalls as
> >> a supervisor entity when walking the pagetables.  The key point here is
> >> that Xen must emulate the hardware pagewalker when trying to find the
> >> data being pointed to.
> >> 
> >> If Xen has a bug causing spurious accesses to HVM guests, then all bets
> >> are already off wrt memory corruption, because real hardware isn't used
> >> to make the access.
> >> 
> >> As indicated before, we could deliberately break the Xen pagewalker to
> >> ignore SMAP.  However, that would be identical to "pretend the guest
> >> actually whitelisted this access". 
> > 
> > I think there is important difference between "whitelist all accesses
> > to guest user memory for the hypercall time" vs "whitelist accesses done
> > through copy_from_guest/copy_to_guest only". If I understand correctly
> > above description, modifying privcmd_call would also suppress #PF when
> > Xen would be tricked to access guest memory outside of copy_*_guest. Am
> > I missing something?
> 
> There are two additional things to consider here:
> 
> 1) For HVM guests, Xen can't access guest memory unintentionally,
> as (other than in the PV case) virtual address spaces are distinct.

Good point.

> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
> intended view, without affecting Xen's. That is, as soon as hypervisor
> context is being entered again, SMAP protection would be in effect
> again (albeit as per point 1 guarding only against accessing PV guest
> mappings).
> 
> So the driver adjustment suggested by Andrew has an effect on only
> page walks done by Xen during copy_{to,from}_guest(), but not on
> actual memory accesses.

Ok, so indeed the kernel patch makes the most sense here. Is the change
in this shape (if works - I'll test it shortly) good to include
upstream, or is it "ugly hack"?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-16 23:06                           ` Marek Marczykowski-Górecki
@ 2017-01-16 23:41                             ` Andrew Cooper
  2017-06-22  8:23                               ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2017-01-16 23:41 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki, Jan Beulich; +Cc: xen-devel

On 16/01/2017 23:06, Marek Marczykowski-Górecki wrote:
> On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
>>>>> On 14.01.17 at 03:52, <marmarek@invisiblethingslab.com> wrote:
>>> On Sat, Jan 14, 2017 at 01:47:49AM +0000, Andrew Cooper wrote:
>>>> In a native situation, SMAP raises #PF if hardware tries to follow a
>>>> user pointer outside of an area whitelisted by the kernel.  (The
>>>> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
>>>>
>>>> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
>>>> (for better or worse, but realistically as a consequence of SMAP being
>>>> ~10 years younger than HVM guests) accesses pointers from hypercalls as
>>>> a supervisor entity when walking the pagetables.  The key point here is
>>>> that Xen must emulate the hardware pagewalker when trying to find the
>>>> data being pointed to.
>>>>
>>>> If Xen has a bug causing spurious accesses to HVM guests, then all bets
>>>> are already off wrt memory corruption, because real hardware isn't used
>>>> to make the access.
>>>>
>>>> As indicated before, we could deliberately break the Xen pagewalker to
>>>> ignore SMAP.  However, that would be identical to "pretend the guest
>>>> actually whitelisted this access". 
>>> I think there is important difference between "whitelist all accesses
>>> to guest user memory for the hypercall time" vs "whitelist accesses done
>>> through copy_from_guest/copy_to_guest only". If I understand correctly
>>> above description, modifying privcmd_call would also suppress #PF when
>>> Xen would be tricked to access guest memory outside of copy_*_guest. Am
>>> I missing something?
>> There are two additional things to consider here:
>>
>> 1) For HVM guests, Xen can't access guest memory unintentionally,
>> as (other than in the PV case) virtual address spaces are distinct.
> Good point.
>
>> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
>> intended view, without affecting Xen's. That is, as soon as hypervisor
>> context is being entered again, SMAP protection would be in effect
>> again (albeit as per point 1 guarding only against accessing PV guest
>> mappings).
>>
>> So the driver adjustment suggested by Andrew has an effect on only
>> page walks done by Xen during copy_{to,from}_guest(), but not on
>> actual memory accesses.
> Ok, so indeed the kernel patch makes the most sense here. Is the change
> in this shape (if works - I'll test it shortly) good to include
> upstream, or is it "ugly hack"?

If it works (which I suspect it will), then it will be the correct
proper upstream fix, and will of course CC stable@.

In the meantime until it percolates into downstream kernels, disabling
SMAP for affected guests is probably the best stopgap solution.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-01-16 23:41                             ` Andrew Cooper
@ 2017-06-22  8:23                               ` Marek Marczykowski-Górecki
  2017-06-22  8:27                                 ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-06-22  8:23 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Jan Beulich, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1432 bytes --]

[resurrecting old thread...]

On Mon, Jan 16, 2017 at 11:41:55PM +0000, Andrew Cooper wrote:
> On 16/01/2017 23:06, Marek Marczykowski-Górecki wrote:
> > On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
> >> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
> >> intended view, without affecting Xen's. That is, as soon as hypervisor
> >> context is being entered again, SMAP protection would be in effect
> >> again (albeit as per point 1 guarding only against accessing PV guest
> >> mappings).
> >>
> >> So the driver adjustment suggested by Andrew has an effect on only
> >> page walks done by Xen during copy_{to,from}_guest(), but not on
> >> actual memory accesses.
> > Ok, so indeed the kernel patch makes the most sense here. Is the change
> > in this shape (if works - I'll test it shortly) good to include
> > upstream, or is it "ugly hack"?
> 
> If it works (which I suspect it will), then it will be the correct
> proper upstream fix, and will of course CC stable@.

Should I submit it?

> In the meantime until it percolates into downstream kernels, disabling
> SMAP for affected guests is probably the best stopgap solution.

How to disable SMAP for selected guests only?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works
  2017-06-22  8:23                               ` Marek Marczykowski-Górecki
@ 2017-06-22  8:27                                 ` Andrew Cooper
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Cooper @ 2017-06-22  8:27 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: Jan Beulich, xen-devel

On 22/06/2017 09:23, Marek Marczykowski-Górecki wrote:
> [resurrecting old thread...]
>
> On Mon, Jan 16, 2017 at 11:41:55PM +0000, Andrew Cooper wrote:
>> On 16/01/2017 23:06, Marek Marczykowski-Górecki wrote:
>>> On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
>>>> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
>>>> intended view, without affecting Xen's. That is, as soon as hypervisor
>>>> context is being entered again, SMAP protection would be in effect
>>>> again (albeit as per point 1 guarding only against accessing PV guest
>>>> mappings).
>>>>
>>>> So the driver adjustment suggested by Andrew has an effect on only
>>>> page walks done by Xen during copy_{to,from}_guest(), but not on
>>>> actual memory accesses.
>>> Ok, so indeed the kernel patch makes the most sense here. Is the change
>>> in this shape (if works - I'll test it shortly) good to include
>>> upstream, or is it "ugly hack"?
>> If it works (which I suspect it will), then it will be the correct
>> proper upstream fix, and will of course CC stable@.
> Should I submit it?

Yes please.

>
>> In the meantime until it percolates into downstream kernels, disabling
>> SMAP for affected guests is probably the best stopgap solution.
> How to disable SMAP for selected guests only?

The toolstack definitely has that kind of control, but I don't know how
well it works in practice in libxl.  You want to look into the CPUID=
configuration option.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-06-22  8:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 17:31 xc_evtchn_status fails with EFAULT on HVM, the same on PV works Marek Marczykowski-Górecki
2017-01-13 17:38 ` Andrew Cooper
2017-01-13 18:03   ` Marek Marczykowski-Górecki
2017-01-13 18:15     ` Andrew Cooper
2017-01-13 18:32       ` Marek Marczykowski-Górecki
2017-01-13 18:37         ` Andrew Cooper
2017-01-13 18:59           ` Marek Marczykowski-Górecki
2017-01-13 19:27             ` Andrew Cooper
2017-01-13 19:40               ` Marek Marczykowski-Górecki
2017-01-13 19:54                 ` Andrew Cooper
2017-01-13 20:32                   ` Marek Marczykowski-Górecki
2017-01-14  1:47                     ` Andrew Cooper
2017-01-14  2:52                       ` Marek Marczykowski-Górecki
2017-01-16 12:17                         ` Jan Beulich
2017-01-16 23:06                           ` Marek Marczykowski-Górecki
2017-01-16 23:41                             ` Andrew Cooper
2017-06-22  8:23                               ` Marek Marczykowski-Górecki
2017-06-22  8:27                                 ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.