xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Corneliu ZUZU <czuzu@bitdefender.com>
To: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED
Date: Wed, 22 Jun 2016 11:34:18 +0300	[thread overview]
Message-ID: <1fadf1db-b592-012b-c1dc-a70fa4be4b7f@bitdefender.com> (raw)
In-Reply-To: <CABfawhkMWG3OyPo8X9G2sgk5DBiHKNMDaE+7STNi26n4+dTZpA@mail.gmail.com>


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

On 6/21/2016 6:09 PM, Tamas K Lengyel wrote:
>
>
> On Jun 21, 2016 05:26, "Corneliu ZUZU" <czuzu@bitdefender.com 
> <mailto:czuzu@bitdefender.com>> wrote:
> >
> > On 6/16/2016 7:11 PM, Tamas K Lengyel wrote:
> >>
> >> On Thu, Jun 16, 2016 at 8:07 AM, Corneliu ZUZU 
> <czuzu@bitdefender.com <mailto:czuzu@bitdefender.com>> wrote:
> >>>
> >>> For VM_EVENT_FLAG_DENY to work, the vcpu must be paused (sync = 1) 
> until the
> >>> vm-event is handled. A vm-event response having VM_EVENT_FLAG_DENY 
> flag set
> >>> should also set the VM_EVENT_FLAG_VCPU_PAUSED flag. Enforce that in
> >>> vm_event_register_write_resume().
> >>
> >> Well, the problem with this is that the user can set the VCPU_PAUSED
> >> flag any time it wants. It can happen that Xen hasn't paused the vCPU
> >> but the user still sends that flag, in which case the unpause the flag
> >> induces will not actually do anything. You should also check if the
> >> vCPU is in fact paused rather then just relying on this flag.
> >>
> >> Tamas
> >>
> >
> > Tamas,
> >
> > Isn't that also the case with the following block @ vm_event_resume:
> >
> >         if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
> >         {
> >             if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
> >                 vm_event_set_registers(v, &rsp);
> >
> >             if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
> >                 vm_event_toggle_singlestep(d, v);
> >
> >             vm_event_vcpu_unpause(v);
> >         }
> >
> > , i.e. shouldn't it be fixed to:
> >
> >         /* check flags which apply only when the vCPU is paused */
> >         if ( atomic_read(&v->vm_event_pause_count) )
> >         {
> >             if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
> >                 vm_event_set_registers(v, &rsp);
> >             if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
> >                 vm_event_toggle_singlestep(d, v);
> >             if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
> >                 vm_event_vcpu_unpause(v);
> >         }
> > ?
> >
> > If this holds, the check for vCPU pause can also be removed from 
> vm_event_toggle_singlestep (maybe turned into an ASSERT which could 
> also be added to vm_event_set_registers).
> >
>
> Yes, reworking that whole part as you outlined above would be nice!
>
> Tamas
>

I've also noticed there's a vm-event vCPU pause count 
(v->vm_event_pause_count), which is synchronized with the global pause 
count (v->pause_count).
Since we rely on vm_event_pause_count to determine if the vCPU is 
paused, I'm wondering if it can't be 'corrupted' by the user.

More specifically, is the code written to ensure that the following 
can't happen:
1. v->vm_event_pause_count > 0 (vm-event subsystem reports that the vCPU 
is paused)
2. the toolstack user can somehow decrement the global v->pause_count 
without decrementing v->vm_event_pause_count (maybe by doing consecutive 
domain resumes)
3. after many decrements by the user (2.),  v->pause_count = 0 (the vCPU 
becomes unpaused), but v->vm_event_pause_count is still > 0
?

Apparently it can't since there's a separate per-domain pause_count as 
well, but the code is a bit involved so I'm just asking to be sure.

Corneliu.

[-- Attachment #1.2: Type: text/html, Size: 5035 bytes --]

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

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

  reply	other threads:[~2016-06-22  8:34 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 14:04 [PATCH 0/7] vm-event: Implement ARM support for control-register writes Corneliu ZUZU
2016-06-16 14:06 ` [PATCH 1/7] minor (formatting) fixes Corneliu ZUZU
2016-06-16 14:24   ` Jan Beulich
2016-06-16 19:19     ` Corneliu ZUZU
2016-06-17  7:06       ` Jan Beulich
2016-06-17 10:46         ` Corneliu ZUZU
2016-06-16 16:02   ` Tamas K Lengyel
2016-06-17  8:33     ` Corneliu ZUZU
2016-06-17  8:36       ` Razvan Cojocaru
2016-06-17  9:29         ` Andrew Cooper
2016-06-17  9:35           ` Jan Beulich
2016-06-17  9:33         ` Jan Beulich
2016-06-17  9:36           ` Razvan Cojocaru
2016-06-17  9:40             ` Jan Beulich
2016-06-17  9:42               ` Razvan Cojocaru
2016-06-17 19:05           ` Tamas K Lengyel
2016-06-16 14:07 ` [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED Corneliu ZUZU
2016-06-16 16:11   ` Tamas K Lengyel
2016-06-17  8:43     ` Corneliu ZUZU
2016-06-21 11:26     ` Corneliu ZUZU
2016-06-21 15:09       ` Tamas K Lengyel
2016-06-22  8:34         ` Corneliu ZUZU [this message]
2016-06-16 14:08 ` [PATCH 3/7] vm-event: introduce vm_event_vcpu_enter Corneliu ZUZU
2016-06-16 14:51   ` Jan Beulich
2016-06-16 20:10     ` Corneliu ZUZU
2016-06-16 20:33       ` Razvan Cojocaru
2016-06-17 10:41         ` Corneliu ZUZU
2016-06-17  7:17       ` Jan Beulich
2016-06-17 11:13         ` Corneliu ZUZU
2016-06-17 11:27           ` Jan Beulich
2016-06-17 12:13             ` Corneliu ZUZU
2016-06-16 16:17   ` Tamas K Lengyel
2016-06-17  9:19     ` Corneliu ZUZU
2016-06-17  8:55   ` Julien Grall
2016-06-17 11:40     ` Corneliu ZUZU
2016-06-17 13:22       ` Julien Grall
2016-06-16 14:09 ` [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly Corneliu ZUZU
2016-06-16 15:00   ` Jan Beulich
2016-06-16 20:20     ` Corneliu ZUZU
2016-06-17  7:20       ` Jan Beulich
2016-06-17 11:23         ` Corneliu ZUZU
2016-06-16 16:27   ` Tamas K Lengyel
2016-06-17  9:24     ` Corneliu ZUZU
2016-06-16 14:10 ` [PATCH 5/7] x86: replace monitor_write_data.do_write with enum Corneliu ZUZU
2016-06-16 14:12 ` [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr Corneliu ZUZU
2016-06-16 15:16   ` Jan Beulich
2016-06-17  8:25     ` Corneliu ZUZU
2016-06-17  8:38       ` Jan Beulich
2016-06-17 11:31         ` Corneliu ZUZU
2016-06-21  7:08       ` Corneliu ZUZU
2016-06-21  7:20         ` Jan Beulich
2016-06-21 15:22           ` Tamas K Lengyel
2016-06-22  6:33             ` Jan Beulich
2016-06-16 16:55   ` Tamas K Lengyel
2016-06-17 10:37     ` Corneliu ZUZU
2016-06-16 14:13 ` [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events Corneliu ZUZU
2016-06-16 14:26   ` Julien Grall
2016-06-16 19:24     ` Corneliu ZUZU
2016-06-16 21:28       ` Julien Grall
2016-06-17 11:46         ` Corneliu ZUZU
2016-06-16 16:49   ` Julien Grall
2016-06-17 10:36     ` Corneliu ZUZU
2016-06-17 13:18       ` Julien Grall
2016-06-22 16:35       ` Corneliu ZUZU
2016-06-22 17:17         ` Julien Grall
2016-06-22 18:39           ` Corneliu ZUZU
2016-06-22 19:37             ` Corneliu ZUZU
2016-06-22 19:41               ` Julien Grall
2016-06-23  5:31                 ` Corneliu ZUZU
2016-06-23  5:49                   ` Corneliu ZUZU
2016-06-23 11:11                     ` Julien Grall
2016-06-24  9:32                       ` Corneliu ZUZU
2016-06-23 11:00           ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1fadf1db-b592-012b-c1dc-a70fa4be4b7f@bitdefender.com \
    --to=czuzu@bitdefender.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=rcojocaru@bitdefender.com \
    --cc=tamas@tklengyel.com \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).