* [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
@ 2016-04-07 8:39 Razvan Cojocaru
2016-04-07 17:27 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Razvan Cojocaru @ 2016-04-07 8:39 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, paul.durrant, keir, Razvan Cojocaru, jbeulich
Theoretically it is possible for mem_access_emulate_each_rep to be
true even when current->arch.vm_event == NULL, so add an extra
check to hvmemul_virtual_to_linear().
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
xen/arch/x86/hvm/emulate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index f5ab5bc..91413d2 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -514,7 +514,7 @@ static int hvmemul_virtual_to_linear(
* vm_event being triggered for repeated writes to a whole page.
*/
if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
- current->arch.vm_event->emulate_flags != 0 )
+ current->arch.vm_event && current->arch.vm_event->emulate_flags != 0 )
max_reps = 1;
/*
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
2016-04-07 8:39 [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear() Razvan Cojocaru
@ 2016-04-07 17:27 ` Jan Beulich
2016-04-07 17:54 ` Razvan Cojocaru
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-04-07 17:27 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: andrew.cooper3, paul.durrant, keir, xen-devel
>>> On 07.04.16 at 10:39, <rcojocaru@bitdefender.com> wrote:
> Theoretically it is possible for mem_access_emulate_each_rep to be
> true even when current->arch.vm_event == NULL, so add an extra
> check to hvmemul_virtual_to_linear().
Mind saying what those theoretical conditions are when this might
happen?
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -514,7 +514,7 @@ static int hvmemul_virtual_to_linear(
> * vm_event being triggered for repeated writes to a whole page.
> */
> if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
> - current->arch.vm_event->emulate_flags != 0 )
> + current->arch.vm_event && current->arch.vm_event->emulate_flags != 0 )
That's then the third instance of "current" here - this needs
latching into a local variable.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
2016-04-07 17:27 ` Jan Beulich
@ 2016-04-07 17:54 ` Razvan Cojocaru
2016-04-07 18:13 ` Razvan Cojocaru
2016-04-07 21:17 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Razvan Cojocaru @ 2016-04-07 17:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, paul.durrant, keir, xen-devel
On 04/07/16 20:27, Jan Beulich wrote:
>>>> On 07.04.16 at 10:39, <rcojocaru@bitdefender.com> wrote:
>> Theoretically it is possible for mem_access_emulate_each_rep to be
>> true even when current->arch.vm_event == NULL, so add an extra
>> check to hvmemul_virtual_to_linear().
>
> Mind saying what those theoretical conditions are when this might
> happen?
This could happen if someone were to call xc_monitor_emulate_each_rep(),
but not xc_monitor_enable() (when current->arch.vm_event gets
allocated), or after someone called both, but afterwards called
xc_monitor_disable() (when current->arch.vm_event gets freed).
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -514,7 +514,7 @@ static int hvmemul_virtual_to_linear(
>> * vm_event being triggered for repeated writes to a whole page.
>> */
>> if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
>> - current->arch.vm_event->emulate_flags != 0 )
>> + current->arch.vm_event && current->arch.vm_event->emulate_flags != 0 )
>
> That's then the third instance of "current" here - this needs
> latching into a local variable.
No problem.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
2016-04-07 17:54 ` Razvan Cojocaru
@ 2016-04-07 18:13 ` Razvan Cojocaru
2016-04-07 21:17 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Razvan Cojocaru @ 2016-04-07 18:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, paul.durrant, keir, xen-devel
On 04/07/16 20:54, Razvan Cojocaru wrote:
> On 04/07/16 20:27, Jan Beulich wrote:
>>>>> On 07.04.16 at 10:39, <rcojocaru@bitdefender.com> wrote:
>>> Theoretically it is possible for mem_access_emulate_each_rep to be
>>> true even when current->arch.vm_event == NULL, so add an extra
>>> check to hvmemul_virtual_to_linear().
>>
>> Mind saying what those theoretical conditions are when this might
>> happen?
>
> This could happen if someone were to call xc_monitor_emulate_each_rep(),
> but not xc_monitor_enable() (when current->arch.vm_event gets
> allocated), or after someone called both, but afterwards called
> xc_monitor_disable() (when current->arch.vm_event gets freed).
Actually, I need to correct myself here: only the first case applies. I
was looking at older Xen source code, but there's a
"d->arch.mem_access_emulate_each_rep = 0;" statement in
vm_event_cleanup_domain() now, so calling xc_monitor_disable() would not
cause a problem.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
2016-04-07 17:54 ` Razvan Cojocaru
2016-04-07 18:13 ` Razvan Cojocaru
@ 2016-04-07 21:17 ` Jan Beulich
2016-04-08 5:32 ` Razvan Cojocaru
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-04-07 21:17 UTC (permalink / raw)
To: Razvan Cojocaru; +Cc: andrew.cooper3, paul.durrant, keir, xen-devel
>>> On 07.04.16 at 19:54, <rcojocaru@bitdefender.com> wrote:
> On 04/07/16 20:27, Jan Beulich wrote:
>>>>> On 07.04.16 at 10:39, <rcojocaru@bitdefender.com> wrote:
>>> Theoretically it is possible for mem_access_emulate_each_rep to be
>>> true even when current->arch.vm_event == NULL, so add an extra
>>> check to hvmemul_virtual_to_linear().
>>
>> Mind saying what those theoretical conditions are when this might
>> happen?
>
> This could happen if someone were to call xc_monitor_emulate_each_rep(),
> but not xc_monitor_enable() (when current->arch.vm_event gets
> allocated), or after someone called both, but afterwards called
> xc_monitor_disable() (when current->arch.vm_event gets freed).
Then wouldn't the correct action be to fail
xc_monitor_emulate_each_rep() (i.e. whatever hypercall this
resolves to) when the monitor is not enabled? (You did already
clarify that the other variant isn't applicable)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear()
2016-04-07 21:17 ` Jan Beulich
@ 2016-04-08 5:32 ` Razvan Cojocaru
0 siblings, 0 replies; 6+ messages in thread
From: Razvan Cojocaru @ 2016-04-08 5:32 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, paul.durrant, keir, xen-devel
On 04/08/16 00:17, Jan Beulich wrote:
>>>> On 07.04.16 at 19:54, <rcojocaru@bitdefender.com> wrote:
>> On 04/07/16 20:27, Jan Beulich wrote:
>>>>>> On 07.04.16 at 10:39, <rcojocaru@bitdefender.com> wrote:
>>>> Theoretically it is possible for mem_access_emulate_each_rep to be
>>>> true even when current->arch.vm_event == NULL, so add an extra
>>>> check to hvmemul_virtual_to_linear().
>>>
>>> Mind saying what those theoretical conditions are when this might
>>> happen?
>>
>> This could happen if someone were to call xc_monitor_emulate_each_rep(),
>> but not xc_monitor_enable() (when current->arch.vm_event gets
>> allocated), or after someone called both, but afterwards called
>> xc_monitor_disable() (when current->arch.vm_event gets freed).
>
> Then wouldn't the correct action be to fail
> xc_monitor_emulate_each_rep() (i.e. whatever hypercall this
> resolves to) when the monitor is not enabled? (You did already
> clarify that the other variant isn't applicable)?
Yes, I think that's better. I'll do that instead.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-08 5:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07 8:39 [PATCH] x86/emulate: Check current->arch.vm_event in hvmemul_virtual_to_linear() Razvan Cojocaru
2016-04-07 17:27 ` Jan Beulich
2016-04-07 17:54 ` Razvan Cojocaru
2016-04-07 18:13 ` Razvan Cojocaru
2016-04-07 21:17 ` Jan Beulich
2016-04-08 5:32 ` Razvan Cojocaru
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).