Re: A possible pointer_overflow in xen-4.13

[Rroach]

After patching it, this works fine and UBSAN dose not have any error report 
about it.


------------------ Original ------------------
From: &nbsp;"Andrew Cooper";<andrew.coop...@citrix.com&gt;;
Send time:&nbsp;Saturday, Jun 26, 2021 9:50 PM
To:&nbsp;"Rroach"<2284696...@qq.com&gt;; 
"xen-devel"<xen-devel@lists.xenproject.org&gt;; 

Subject: &nbsp;Re: A possible pointer_overflow in xen-4.13



           On 26/06/2021 14:29, Rroach wrote:
     
                              Hi, I compile Xen-4.13 with CONFIG_UBSAN, and try 
test             it. However, during testing, xl dmesg got the output as        
     shown below.
           
           
           It seems that there is a potential pointer overflow             
within arch/x86/pv/emul-priv-op.c:131 where xen try to             execute 
instruction ''' APPEND_CALL(save_guest_gprs)             '''??where APPEND_CALL 
try to add an offset on *p without             proper checking.
           
           
           I compiled xen-4.13 by clang-9, with following             
instructions: ''' export CONFIG_UBSAN=y ''' &amp;&amp; '''             make 
clang=y debug=y ''' . Do you have any idea what going             on here?
         
          
     You say Xen 4.13, but APPEND_CALL() doesn't exist       there.&nbsp; I 
added it in 4.14 when I rewrote this mess to be       compatible with CET by 
not using a ROP gadget.&nbsp; Your backtrace       says 4.15 unstable which 
means its an old staging build (not that       that is going to have any effect 
on this specific issue).
       
       The fact that it continued executing correctly means the       
calculation did the right thing, whether or not UBSAN was happy.        The 
displacement will end up negative as the stub we're writing is       
numerically higher than {load,save}_guest_gprs(), which I guess       means 
that f - stub_va will underflow.
       
       I'm very confused as to why UBSAN reports against       
save_guest_gprs() considering that load_guest_gprs() when through       the 
exact same logic a few instructions earlier.
       
       Either way, does this make the problem go away?
       
       diff --git a/xen/arch/x86/pv/emul-priv-op.c       
b/xen/arch/x86/pv/emul-priv-op.c
       index 11467a1e3a..be41bced76 100644
       --- a/xen/arch/x86/pv/emul-priv-op.c
       +++ b/xen/arch/x86/pv/emul-priv-op.c
       @@ -98,7 +98,7 @@ static io_emul_stub_t *io_emul_stub_setup(struct       
priv_op_ctxt *ctxt, u8 opcode,
       &nbsp;#define APPEND_BUFF(b) ({ memcpy(p, b, sizeof(b)); p +=       
sizeof(b); })
       &nbsp;#define       
APPEND_CALL(f)&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 &nbsp; \
       &nbsp;&nbsp;&nbsp;        
({&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
        \
       -&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; long disp = (long)(f) - (stub_va 
+ p -       ctxt-&gt;io_emul_stub + 5); \
       +&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; long disp = (long)(f) - 
(long)(stub_va + p -       ctxt-&gt;io_emul_stub + 5); \
       &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; BUG_ON((int32_t)disp !=      
 
disp);&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 &nbsp; \
       &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; *p++ =       
0xe8;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 &nbsp; \
       &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; *(int32_t *)p = disp; p +=   
    
4;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 &nbsp; \
       
       ~Andrew