diff mbox

[v2] x86/hvm: Fix altp2m_vcpu_enable_notify error handling

Message ID 20171115134759.14398-1-apop@bitdefender.com (mailing list archive)
State New, archived
Headers show

Commit Message

Adrian Pop Nov. 15, 2017, 1:47 p.m. UTC 
The altp2m_vcpu_enable_notify subop handler might skip calling
rcu_unlock_domain() after rcu_lock_current_domain().  Albeit since both
rcu functions are no-ops when run on the current domain, this doesn't
really have repercussions.

The second change is adding a missing break that would have potentially
enabled #VE for the current domain even if it had intended to enable it
for another one (not a supported functionality).

Signed-off-by: Adrian Pop <apop@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
changes in v2:
- reword the commit message
---
 xen/arch/x86/hvm/hvm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Jan Beulich Nov. 15, 2017, 2:10 p.m. UTC | #1 
>>> On 15.11.17 at 14:47, <apop@bitdefender.com> wrote:
> The altp2m_vcpu_enable_notify subop handler might skip calling
> rcu_unlock_domain() after rcu_lock_current_domain().  Albeit since both
> rcu functions are no-ops when run on the current domain, this doesn't
> really have repercussions.
> 
> The second change is adding a missing break that would have potentially
> enabled #VE for the current domain even if it had intended to enable it
> for another one (not a supported functionality).

Thanks, much better.

> Signed-off-by: Adrian Pop <apop@bitdefender.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Andrew Cooper Nov. 15, 2017, 2:16 p.m. UTC | #2 
On 15/11/17 14:10, Jan Beulich wrote:
>>>> On 15.11.17 at 14:47, <apop@bitdefender.com> wrote:
>> The altp2m_vcpu_enable_notify subop handler might skip calling
>> rcu_unlock_domain() after rcu_lock_current_domain().  Albeit since both
>> rcu functions are no-ops when run on the current domain, this doesn't
>> really have repercussions.
>>
>> The second change is adding a missing break that would have potentially
>> enabled #VE for the current domain even if it had intended to enable it
>> for another one (not a supported functionality).
> Thanks, much better.
>
>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

FOAD, Requesting a release ack for this change.

~Andrew
Julien Grall Nov. 16, 2017, 2:57 p.m. UTC | #3 
Hi,

On 15/11/17 14:16, Andrew Cooper wrote:
> On 15/11/17 14:10, Jan Beulich wrote:
>>>>> On 15.11.17 at 14:47, <apop@bitdefender.com> wrote:
>>> The altp2m_vcpu_enable_notify subop handler might skip calling
>>> rcu_unlock_domain() after rcu_lock_current_domain().  Albeit since both
>>> rcu functions are no-ops when run on the current domain, this doesn't
>>> really have repercussions.
>>>
>>> The second change is adding a missing break that would have potentially
>>> enabled #VE for the current domain even if it had intended to enable it
>>> for another one (not a supported functionality).
>> Thanks, much better.
>>
>>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> FOAD, Requesting a release ack for this change.

Release-acked-by: Julien Grall <julien.grall@linaro.org>

Cheers,
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 205b4cb685..0af498a312 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4534,12 +4534,18 @@  static int do_altp2m_op(
 
         if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
              a.u.enable_notify.vcpu_id != curr->vcpu_id )
+        {
             rc = -EINVAL;
+            break;
+        }
 
         if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
              mfn_eq(get_gfn_query_unlocked(curr->domain,
                     a.u.enable_notify.gfn, &p2mt), INVALID_MFN) )
-            return -EINVAL;
+        {
+            rc = -EINVAL;
+            break;
+        }
 
         vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
         altp2m_vcpu_update_vmfunc_ve(curr);