[Xen-changelog] [xen stable-4.5] x86/hvmloader: avoid data corruption with xenstore reads/writes

[patchbot@xxxxxxx]

commit 8c166421b0af090813ad2cc691fad24784feaf1e
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Tue Jul 21 11:11:19 2015 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Jul 21 11:11:19 2015 +0200

    x86/hvmloader: avoid data corruption with xenstore reads/writes
    
    The functions ring_read and ring_write() have logic to try and deal with
    partial reads and writes.
    
    However, in all cases where the "while (len)" loop executed twice, data
    corruption would occur as the second memcpy() starts from the beginning of
    "data" again, rather than from where it got to.
    
    This bug manifested itself as protocol corruption when a reply header 
crossed
    the first wrap of the response ring.  However, similar corruption would also
    occur if hvmloader observed xenstored performing partial writes of the block
    in question, or if hvmloader had to wait for xenstored to make space in 
either
    ring.
    
    Reported-by: Adam Kucia <djexit@xxxxx>
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    master commit: bbbe7e7157a964c485fb861765be291734676932
    master date: 2015-07-07 14:39:27 +0200
---
 tools/firmware/hvmloader/xenbus.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/firmware/hvmloader/xenbus.c 
b/tools/firmware/hvmloader/xenbus.c
index f900a1e..00513f7 100644
--- a/tools/firmware/hvmloader/xenbus.c
+++ b/tools/firmware/hvmloader/xenbus.c
@@ -105,7 +105,7 @@ void xenbus_shutdown(void)
 /* Helper functions: copy data in and out of the ring */
 static void ring_write(const char *data, uint32_t len)
 {
-    uint32_t part;
+    uint32_t part, done = 0;
 
     ASSERT(len <= XENSTORE_PAYLOAD_MAX);
 
@@ -122,16 +122,18 @@ static void ring_write(const char *data, uint32_t len)
         if ( part > len ) 
             part = len;
 
-        memcpy(rings->req + MASK_XENSTORE_IDX(rings->req_prod), data, part);
+        memcpy(rings->req + MASK_XENSTORE_IDX(rings->req_prod),
+               data + done, part);
         barrier(); /* = wmb before prod write, rmb before next cons read */
         rings->req_prod += part;
         len -= part;
+        done += part;
     }
 }
 
 static void ring_read(char *data, uint32_t len)
 {
-    uint32_t part;
+    uint32_t part, done = 0;
 
     ASSERT(len <= XENSTORE_PAYLOAD_MAX);
 
@@ -148,10 +150,12 @@ static void ring_read(char *data, uint32_t len)
         if ( part > len )
             part = len;
 
-        memcpy(data, rings->rsp + MASK_XENSTORE_IDX(rings->rsp_cons), part);
+        memcpy(data + done,
+               rings->rsp + MASK_XENSTORE_IDX(rings->rsp_cons), part);
         barrier(); /* = wmb before cons write, rmb before next prod read */
         rings->rsp_cons += part;
         len -= part;
+        done += part;
     }
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.5

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog