[PATCH] x86/nHVM: avoid NULL deref during INVLPG intercept handling

[PATCH] x86/nHVM: avoid NULL deref during INVLPG intercept handling

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

When intercepting (or emulating) L1 guest INVLPG, the nested P2M
pointer may be (is?) NULL, and hence there's no point in calling
p2m_flush(). In fact doing so would cause a dereference of that NULL
pointer at least in the ASSERT() right at the beginning of the
function.

While so far nothing supports hap_invlpg() being reachable from the
INVLPG intercept paths (only INVLPG insn emulation would lead there),
and hence the code in question (added by dd6de3ab99 ["Implement
Nested-on-Nested"]) appears to be dead, this seems to be the change
which can be agreed on as an immediate fix. Ideally, however, the
problematic code would go away altogether. See thread at
lists.xenproject.org/archives/html/xen-devel/2016-01/msg03762.html.

Reported-by: 刘令 <liuling-it@360.cn>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -687,7 +687,8 @@ static bool_t hap_invlpg(struct vcpu *v,
          * Must perform the flush right now or an other vcpu may
          * use it when we use the next VMRUN emulation, otherwise.
          */
-        p2m_flush(v, vcpu_nestedhvm(v).nv_p2m);
+        if ( vcpu_nestedhvm(v).nv_p2m )
+            p2m_flush(v, vcpu_nestedhvm(v).nv_p2m);
         return 1;
     }




[-- Attachment #2: x86-nHVM-invlpg-crash.patch --]
[-- Type: text/plain, Size: 1361 bytes --]

x86/nHVM: avoid NULL deref during INVLPG intercept handling

When intercepting (or emulating) L1 guest INVLPG, the nested P2M
pointer may be (is?) NULL, and hence there's no point in calling
p2m_flush(). In fact doing so would cause a dereference of that NULL
pointer at least in the ASSERT() right at the beginning of the
function.

While so far nothing supports hap_invlpg() being reachable from the
INVLPG intercept paths (only INVLPG insn emulation would lead there),
and hence the code in question (added by dd6de3ab99 ["Implement
Nested-on-Nested"]) appears to be dead, this seems to be the change
which can be agreed on as an immediate fix. Ideally, however, the
problematic code would go away altogether. See thread at
lists.xenproject.org/archives/html/xen-devel/2016-01/msg03762.html.
   
Reported-by: 刘令 <liuling-it@360.cn>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -687,7 +687,8 @@ static bool_t hap_invlpg(struct vcpu *v,
          * Must perform the flush right now or an other vcpu may
          * use it when we use the next VMRUN emulation, otherwise.
          */
-        p2m_flush(v, vcpu_nestedhvm(v).nv_p2m);
+        if ( vcpu_nestedhvm(v).nv_p2m )
+            p2m_flush(v, vcpu_nestedhvm(v).nv_p2m);
         return 1;
     }
 

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

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

Re: [PATCH] x86/nHVM: avoid NULL deref during INVLPG intercept handling

[Andrew Cooper]

On 05/02/16 07:37, Jan Beulich wrote:
> When intercepting (or emulating) L1 guest INVLPG, the nested P2M
> pointer may be (is?) NULL, and hence there's no point in calling
> p2m_flush(). In fact doing so would cause a dereference of that NULL
> pointer at least in the ASSERT() right at the beginning of the
> function.
>
> While so far nothing supports hap_invlpg() being reachable from the
> INVLPG intercept paths (only INVLPG insn emulation would lead there),
> and hence the code in question (added by dd6de3ab99 ["Implement
> Nested-on-Nested"]) appears to be dead, this seems to be the change
> which can be agreed on as an immediate fix. Ideally, however, the
> problematic code would go away altogether. See thread at
> lists.xenproject.org/archives/html/xen-devel/2016-01/msg03762.html.
>    

Trailing whitespace in this newline.

> Reported-by: 刘令 <liuling-it@360.cn>

There appears to be a unicode error in the email here, but the attached
patch appears correct.

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

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH] x86/nHVM: avoid NULL deref during INVLPG intercept handling

[George Dunlap]

On 05/02/16 07:37, Jan Beulich wrote:
> When intercepting (or emulating) L1 guest INVLPG, the nested P2M
> pointer may be (is?) NULL, and hence there's no point in calling
> p2m_flush(). In fact doing so would cause a dereference of that NULL
> pointer at least in the ASSERT() right at the beginning of the
> function.
> 
> While so far nothing supports hap_invlpg() being reachable from the
> INVLPG intercept paths (only INVLPG insn emulation would lead there),
> and hence the code in question (added by dd6de3ab99 ["Implement
> Nested-on-Nested"]) appears to be dead, this seems to be the change
> which can be agreed on as an immediate fix. Ideally, however, the
> problematic code would go away altogether. See thread at
> lists.xenproject.org/archives/html/xen-devel/2016-01/msg03762.html.
>    
> Reported-by: 刘令 <liuling-it@360.cn>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>



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