Skip to contentSkip to conversation

x86/hvm: Fix altp2m_vcpu_enable_notify error handling

PATCH In review Adrian Pop 4 participants 4 messages Nov 15, 2017 – Nov 16, 2017

lore
Share & tools
lore

Adrian Pop

author
#0 lore raw

PATCH v2 1/1 x86/hvm: Fix altp2m_vcpu_enable_notify error handling

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 <[email protected]>
Reviewed-by Andrew Cooper <[email protected]>

---
changes in v2:
- reword the commit message

---
 xen/arch/x86/hvm/hvm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
xen/arch/x86/hvm/hvm.c +7 -1
index 205b4cb685..0af498a312 100644
@@ -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);
Signature · 7 lines
2.15.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Reviewed-by on v2 1/1
#1 lore raw
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 15.11.17 at 14:47, <[email protected]> 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.

Quoted 2 lines
Signed-off-by Adrian Pop <[email protected]>
Reviewed-by Andrew Cooper <[email protected]>
Reviewed-by Jan Beulich <[email protected]>

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

#2 lore raw
Quoted 6 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · On 15.11.17 at 14:47, <[email protected]> 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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 2 lines
Signed-off-by Adrian Pop <[email protected]>
Reviewed-by Andrew Cooper <[email protected]>
Reviewed-by Jan Beulich <[email protected]>

FOAD, Requesting a release ack for this change.

~Andrew

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

#3 lore raw

Hi,

Quoted 10 lines · Andrew Cooper
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 15.11.17 at 14:47, <[email protected]> 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).

Quoted 7 lines
Quoted 5 lines

Thanks, much better.

Quoted 2 lines
Signed-off-by Adrian Pop <[email protected]>
Reviewed-by Andrew Cooper <[email protected]>
Reviewed-by Jan Beulich <[email protected]>

FOAD, Requesting a release ack for this change.

Release-acked-by Julien Grall <[email protected]>

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel