[PATCH v2 1/3] xc/tmem: Free temporary buffer used during migration

[PATCH v2 1/3] xc/tmem: Free temporary buffer used during migration

[Bob Liu]

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

CID 1090388.

Within the loop reading the pool_id we set the buf:

 if ( (buf = realloc(buf,bufsize)) == NULL )

and then continue on using it without freeing. Worst yet
there are multiple 'if (..) return -1' which do not free
the buffer.

As such insert a 'fail' goto label to free the buffer
and also add on the OK path a mechanism to free the buffer.

Replace all of the 'return -1' with a jump to the failed
label.

v2:
* Remove superfluous braces. (Andrew)

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Bob Liu <bob.liu@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_tmem.c |   44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 3261e10..6832bbe 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -215,6 +215,7 @@ int xc_tmem_save(xc_interface *xch,
     uint32_t pool_id;
     uint32_t minusone = -1;
     struct tmem_handle *h;
+    char *buf = NULL;
 
     if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,0,NULL) <= 0 )
         return 0;
@@ -249,7 +250,6 @@ int xc_tmem_save(xc_interface *xch,
         uint64_t uuid[2];
         uint32_t n_pages;
         uint32_t pagesize;
-        char *buf = NULL;
         int bufsize = 0;
         int checksum = 0;
 
@@ -263,13 +263,13 @@ int xc_tmem_save(xc_interface *xch,
                 n_pages = 0;
             (void)xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,0,&uuid);
             if ( write_exact(io_fd, &pool_id, sizeof(pool_id)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &flags, sizeof(flags)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &n_pages, sizeof(n_pages)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &uuid, sizeof(uuid)) )
-                return -1;
+                goto failed;
             if ( n_pages == 0 )
                 continue;
 
@@ -279,7 +279,7 @@ int xc_tmem_save(xc_interface *xch,
             {
                 bufsize = pagesize + sizeof(struct tmem_handle);
                 if ( (buf = realloc(buf,bufsize)) == NULL )
-                    return -1;
+                    goto failed;
             }
             for ( j = n_pages; j > 0; j-- )
             {
@@ -290,13 +290,13 @@ int xc_tmem_save(xc_interface *xch,
                 {
                     h = (struct tmem_handle *)buf;
                     if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
-                        return -1;
+                        goto failed;
                     if ( write_exact(io_fd, &h->index, sizeof(h->index)) )
-                        return -1;
+                        goto failed;
                     h++;
                     checksum += *(char *)h;
                     if ( write_exact(io_fd, h, pagesize) )
-                        return -1;
+                        goto failed;
                 } else if ( ret == 0 ) {
                     continue;
                 } else {
@@ -304,7 +304,7 @@ int xc_tmem_save(xc_interface *xch,
                     h = (struct tmem_handle *)buf;
                     h->oid[0] = h->oid[1] = h->oid[2] = -1L;
                     if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
-                        return -1;
+                        goto failed;
                     break;
                 }
             }
@@ -315,9 +315,13 @@ int xc_tmem_save(xc_interface *xch,
     /* pool list terminator */
     minusone = -1;
     if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
-        return -1;
+        goto failed;
 
+    free(buf);
     return 1;
+failed:
+    free(buf);
+    return -1;
 }
 
 /* only called for live migration */
@@ -386,6 +390,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
     uint32_t minusone;
     uint32_t weight, cap, flags;
     int checksum = 0;
+    char *buf = NULL;
 
     save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,0,NULL);
     if ( save_version == -1 )
@@ -423,7 +428,6 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
     {
         uint64_t uuid[2];
         uint32_t n_pages;
-        char *buf = NULL;
         int bufsize = 0, pagesize;
         int j;
 
@@ -445,7 +449,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
         {
             bufsize = pagesize;
             if ( (buf = realloc(buf,bufsize)) == NULL )
-                return -1;
+                goto failed;
         }
         for ( j = n_pages; j > 0; j-- )
         {
@@ -453,20 +457,20 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
             uint32_t index;
             int rc;
             if ( read_exact(io_fd, &oid, sizeof(oid)) )
-                return -1;
+                goto failed;
             if ( oid.oid[0] == -1L && oid.oid[1] == -1L && oid.oid[2] == -1L )
                 break;
             if ( read_exact(io_fd, &index, sizeof(index)) )
-                return -1;
+                goto failed;
             if ( read_exact(io_fd, buf, pagesize) )
-                return -1;
+                goto failed;
             checksum += *buf;
             if ( (rc = xc_tmem_control_oid(xch, pool_id,
                                            TMEMC_RESTORE_PUT_PAGE, dom,
                                            bufsize, index, oid, buf)) <= 0 )
             {
                 DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
-                return -1;
+                goto failed;
             }
         }
         if ( n_pages )
@@ -474,9 +478,13 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
                     n_pages-j,dom,pool_id,checksum);
     }
     if ( pool_id != -1 )
-        return -1;
+        goto failed;
 
+    free(buf);
     return 0;
+failed:
+    free(buf);
+    return -1;
 }
 
 /* only called for live migration, must be called after suspend */
-- 
1.7.10.4

[PATCH v2 2/3] xc/tmem: Unchecked return value

[Bob Liu]

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

CID 1055045 complains that the return value needs checking.

The tmem op is a flush call and there is no expectation
right now of any return besides zero. But just in case
this changes lets blow up.

v2:
* superfluous braces and missing space (Andrew)
* Add a comment why bailing out on xc_tmem_save_done is not needed

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
 tools/libxc/xc_domain_save.c |    6 ++++--
 tools/libxc/xc_tmem.c        |    4 ++--
 tools/libxc/xenctrl.h        |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index 71f9b59..856eaa5 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -2107,8 +2107,10 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
         goto copypages;
     }
 
-    if ( tmem_saved != 0 && live )
-        xc_tmem_save_done(xch, dom);
+    if ( tmem_saved != 0 && live && xc_tmem_save_done(xch, dom) )
+        /* N.B. No need to bail out - we leak memory, but that is all assigned
+         * to the zombie guest. */
+       DPRINTF("Warning - failed to flush tmem on originating server.");
 
     if ( live )
     {
diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 6832bbe..250e438 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -355,9 +355,9 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
 }
 
 /* only called for live migration */
-void xc_tmem_save_done(xc_interface *xch, int dom)
+int xc_tmem_save_done(xc_interface *xch, int dom)
 {
-    xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,0,NULL);
+    return xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,0,NULL);
 }
 
 /* restore routines */
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 02129f7..23630a7 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2011,7 +2011,7 @@ int xc_tmem_control(xc_interface *xch,
 int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int arg1);
 int xc_tmem_save(xc_interface *xch, int dom, int live, int fd, int field_marker);
 int xc_tmem_save_extra(xc_interface *xch, int dom, int fd, int field_marker);
-void xc_tmem_save_done(xc_interface *xch, int dom);
+int xc_tmem_save_done(xc_interface *xch, int dom);
 int xc_tmem_restore(xc_interface *xch, int dom, int fd);
 int xc_tmem_restore_extra(xc_interface *xch, int dom, int fd);
 
-- 
1.7.10.4

[PATCH v2 3/3] tmem: fix Out-of-bounds read reported by Coverity

[Bob Liu]

CID 1198729, CID 1198730 and CID 1198734 complain about
"Out-of-bounds read".

This patch fixes them by casting the 'firstbyte' to (uint8_t), some
unnecessary assertion also be dropped.

Signed-off-by: Bob Liu <bob.liu@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/common/tmem.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index f2dc26e..93235c6 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -399,7 +399,7 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
 {
     struct tmem_page_content_descriptor *pcd = pgp->pcd;
     struct page_info *pfp = pgp->pcd->pfp;
-    uint16_t firstbyte = pgp->firstbyte;
+    uint8_t firstbyte = pgp->firstbyte;
     char *pcd_tze = pgp->pcd->tze;
     pagesize_t pcd_size = pcd->size;
     pagesize_t pgp_size = pgp->size;
@@ -407,8 +407,6 @@ static void pcd_disassociate(struct tmem_page_descriptor *pgp, struct tmem_pool
     pagesize_t pcd_csize = pgp->pcd->size;
 
     ASSERT(tmem_dedup_enabled());
-    ASSERT(firstbyte != NOT_SHAREABLE);
-    ASSERT(firstbyte < 256);
 
     if ( have_pcd_rwlock )
         ASSERT_WRITELOCK(&pcd_tree_rwlocks[firstbyte]);
@@ -1231,7 +1229,7 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
     struct tmem_object_root *obj = pgp->us.obj;
     struct tmem_pool *pool = obj->pool;
     struct client *client = pool->client;
-    uint16_t firstbyte = pgp->firstbyte;
+    uint8_t firstbyte = pgp->firstbyte;
 
     if ( pool->is_dying )
         return 0;
@@ -1239,10 +1237,9 @@ static bool_t tmem_try_to_evict_pgp(struct tmem_page_descriptor *pgp, bool_t *ho
     {
         if ( tmem_dedup_enabled() )
         {
-            firstbyte = pgp->firstbyte;
-            if ( firstbyte ==  NOT_SHAREABLE )
+            if ( pgp->firstbyte ==  NOT_SHAREABLE )
                 goto obj_unlock;
-            ASSERT(firstbyte < 256);
+            firstbyte = pgp->firstbyte;
             if ( !write_trylock(&pcd_tree_rwlocks[firstbyte]) )
                 goto obj_unlock;
             if ( pgp->pcd->pgp_ref_count > 1 && !pgp->eviction_attempted )
-- 
1.7.10.4

Re: [PATCH v2 1/3] xc/tmem: Free temporary buffer used during migration

[Jan Beulich]

>>> On 07.05.14 at 08:30, <lliubbo@gmail.com> wrote:
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> CID 1090388.
> 
> Within the loop reading the pool_id we set the buf:
> 
>  if ( (buf = realloc(buf,bufsize)) == NULL )
> 
> and then continue on using it without freeing. Worst yet
> there are multiple 'if (..) return -1' which do not free
> the buffer.
> 
> As such insert a 'fail' goto label to free the buffer
> and also add on the OK path a mechanism to free the buffer.
> 
> Replace all of the 'return -1' with a jump to the failed
> label.
> 
> v2:
> * Remove superfluous braces. (Andrew)
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxc/xc_tmem.c |   44 ++++++++++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 18 deletions(-)

For this and the second patch you ought to Cc the tools maintainers
instead of the hypervisor ones, and for the third patch Cc-ing the
tmem maintainer would be sufficient.

Jan

Re: [PATCH v2 3/3] tmem: fix Out-of-bounds read reported by Coverity

[Jan Beulich]

>>> On 07.05.14 at 08:30, <lliubbo@gmail.com> wrote:
> CID 1198729, CID 1198730 and CID 1198734 complain about
> "Out-of-bounds read".
> 
> This patch fixes them by casting the 'firstbyte' to (uint8_t), some
> unnecessary assertion also be dropped.
> 
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

It looks to me as if the code was more correct without these changes,
and doing these changes just to work around a pretty obvious
Coverity flaw seems rather odd.

Jan

Re: [PATCH v2 1/3] xc/tmem: Free temporary buffer used during migration

[Bob Liu]

Added the tools maintainers, thanks for Jan's reminding.

On 05/07/2014 02:30 PM, Bob Liu wrote:
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> CID 1090388.
> 
> Within the loop reading the pool_id we set the buf:
> 
>  if ( (buf = realloc(buf,bufsize)) == NULL )
> 
> and then continue on using it without freeing. Worst yet
> there are multiple 'if (..) return -1' which do not free
> the buffer.
> 
> As such insert a 'fail' goto label to free the buffer
> and also add on the OK path a mechanism to free the buffer.
> 
> Replace all of the 'return -1' with a jump to the failed
> label.
> 
> v2:
> * Remove superfluous braces. (Andrew)
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxc/xc_tmem.c |   44 ++++++++++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
> index 3261e10..6832bbe 100644
> --- a/tools/libxc/xc_tmem.c
> +++ b/tools/libxc/xc_tmem.c
> @@ -215,6 +215,7 @@ int xc_tmem_save(xc_interface *xch,
>      uint32_t pool_id;
>      uint32_t minusone = -1;
>      struct tmem_handle *h;
> +    char *buf = NULL;
>  
>      if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,0,NULL) <= 0 )
>          return 0;
> @@ -249,7 +250,6 @@ int xc_tmem_save(xc_interface *xch,
>          uint64_t uuid[2];
>          uint32_t n_pages;
>          uint32_t pagesize;
> -        char *buf = NULL;
>          int bufsize = 0;
>          int checksum = 0;
>  
> @@ -263,13 +263,13 @@ int xc_tmem_save(xc_interface *xch,
>                  n_pages = 0;
>              (void)xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,0,&uuid);
>              if ( write_exact(io_fd, &pool_id, sizeof(pool_id)) )
> -                return -1;
> +                goto failed;
>              if ( write_exact(io_fd, &flags, sizeof(flags)) )
> -                return -1;
> +                goto failed;
>              if ( write_exact(io_fd, &n_pages, sizeof(n_pages)) )
> -                return -1;
> +                goto failed;
>              if ( write_exact(io_fd, &uuid, sizeof(uuid)) )
> -                return -1;
> +                goto failed;
>              if ( n_pages == 0 )
>                  continue;
>  
> @@ -279,7 +279,7 @@ int xc_tmem_save(xc_interface *xch,
>              {
>                  bufsize = pagesize + sizeof(struct tmem_handle);
>                  if ( (buf = realloc(buf,bufsize)) == NULL )
> -                    return -1;
> +                    goto failed;
>              }
>              for ( j = n_pages; j > 0; j-- )
>              {
> @@ -290,13 +290,13 @@ int xc_tmem_save(xc_interface *xch,
>                  {
>                      h = (struct tmem_handle *)buf;
>                      if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
> -                        return -1;
> +                        goto failed;
>                      if ( write_exact(io_fd, &h->index, sizeof(h->index)) )
> -                        return -1;
> +                        goto failed;
>                      h++;
>                      checksum += *(char *)h;
>                      if ( write_exact(io_fd, h, pagesize) )
> -                        return -1;
> +                        goto failed;
>                  } else if ( ret == 0 ) {
>                      continue;
>                  } else {
> @@ -304,7 +304,7 @@ int xc_tmem_save(xc_interface *xch,
>                      h = (struct tmem_handle *)buf;
>                      h->oid[0] = h->oid[1] = h->oid[2] = -1L;
>                      if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
> -                        return -1;
> +                        goto failed;
>                      break;
>                  }
>              }
> @@ -315,9 +315,13 @@ int xc_tmem_save(xc_interface *xch,
>      /* pool list terminator */
>      minusone = -1;
>      if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
> -        return -1;
> +        goto failed;
>  
> +    free(buf);
>      return 1;
> +failed:
> +    free(buf);
> +    return -1;
>  }
>  
>  /* only called for live migration */
> @@ -386,6 +390,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
>      uint32_t minusone;
>      uint32_t weight, cap, flags;
>      int checksum = 0;
> +    char *buf = NULL;
>  
>      save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,0,NULL);
>      if ( save_version == -1 )
> @@ -423,7 +428,6 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
>      {
>          uint64_t uuid[2];
>          uint32_t n_pages;
> -        char *buf = NULL;
>          int bufsize = 0, pagesize;
>          int j;
>  
> @@ -445,7 +449,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
>          {
>              bufsize = pagesize;
>              if ( (buf = realloc(buf,bufsize)) == NULL )
> -                return -1;
> +                goto failed;
>          }
>          for ( j = n_pages; j > 0; j-- )
>          {
> @@ -453,20 +457,20 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
>              uint32_t index;
>              int rc;
>              if ( read_exact(io_fd, &oid, sizeof(oid)) )
> -                return -1;
> +                goto failed;
>              if ( oid.oid[0] == -1L && oid.oid[1] == -1L && oid.oid[2] == -1L )
>                  break;
>              if ( read_exact(io_fd, &index, sizeof(index)) )
> -                return -1;
> +                goto failed;
>              if ( read_exact(io_fd, buf, pagesize) )
> -                return -1;
> +                goto failed;
>              checksum += *buf;
>              if ( (rc = xc_tmem_control_oid(xch, pool_id,
>                                             TMEMC_RESTORE_PUT_PAGE, dom,
>                                             bufsize, index, oid, buf)) <= 0 )
>              {
>                  DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
> -                return -1;
> +                goto failed;
>              }
>          }
>          if ( n_pages )
> @@ -474,9 +478,13 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
>                      n_pages-j,dom,pool_id,checksum);
>      }
>      if ( pool_id != -1 )
> -        return -1;
> +        goto failed;
>  
> +    free(buf);
>      return 0;
> +failed:
> +    free(buf);
> +    return -1;
>  }
>  
>  /* only called for live migration, must be called after suspend */
> 

-- 
Regards,
-Bob

Re: [PATCH v2 2/3] xc/tmem: Unchecked return value

[Bob Liu]

CC'd tools maintainers.

On 05/07/2014 02:30 PM, Bob Liu wrote:
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> CID 1055045 complains that the return value needs checking.
> 
> The tmem op is a flush call and there is no expectation
> right now of any return besides zero. But just in case
> this changes lets blow up.
> 
> v2:
> * superfluous braces and missing space (Andrew)
> * Add a comment why bailing out on xc_tmem_save_done is not needed
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> ---
>  tools/libxc/xc_domain_save.c |    6 ++++--
>  tools/libxc/xc_tmem.c        |    4 ++--
>  tools/libxc/xenctrl.h        |    2 +-
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> index 71f9b59..856eaa5 100644
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -2107,8 +2107,10 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
>          goto copypages;
>      }
>  
> -    if ( tmem_saved != 0 && live )
> -        xc_tmem_save_done(xch, dom);
> +    if ( tmem_saved != 0 && live && xc_tmem_save_done(xch, dom) )
> +        /* N.B. No need to bail out - we leak memory, but that is all assigned
> +         * to the zombie guest. */
> +       DPRINTF("Warning - failed to flush tmem on originating server.");
>  
>      if ( live )
>      {
> diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
> index 6832bbe..250e438 100644
> --- a/tools/libxc/xc_tmem.c
> +++ b/tools/libxc/xc_tmem.c
> @@ -355,9 +355,9 @@ int xc_tmem_save_extra(xc_interface *xch, int dom, int io_fd, int field_marker)
>  }
>  
>  /* only called for live migration */
> -void xc_tmem_save_done(xc_interface *xch, int dom)
> +int xc_tmem_save_done(xc_interface *xch, int dom)
>  {
> -    xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,0,NULL);
> +    return xc_tmem_control(xch,0,TMEMC_SAVE_END,dom,0,0,0,NULL);
>  }
>  
>  /* restore routines */
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 02129f7..23630a7 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -2011,7 +2011,7 @@ int xc_tmem_control(xc_interface *xch,
>  int xc_tmem_auth(xc_interface *xch, int cli_id, char *uuid_str, int arg1);
>  int xc_tmem_save(xc_interface *xch, int dom, int live, int fd, int field_marker);
>  int xc_tmem_save_extra(xc_interface *xch, int dom, int fd, int field_marker);
> -void xc_tmem_save_done(xc_interface *xch, int dom);
> +int xc_tmem_save_done(xc_interface *xch, int dom);
>  int xc_tmem_restore(xc_interface *xch, int dom, int fd);
>  int xc_tmem_restore_extra(xc_interface *xch, int dom, int fd);
>  
> 

-- 
Regards,
-Bob

Re: [PATCH v2 3/3] tmem: fix Out-of-bounds read reported by Coverity

[Bob Liu]

On 05/07/2014 04:33 PM, Jan Beulich wrote:
>>>> On 07.05.14 at 08:30, <lliubbo@gmail.com> wrote:
>> CID 1198729, CID 1198730 and CID 1198734 complain about
>> "Out-of-bounds read".
>>
>> This patch fixes them by casting the 'firstbyte' to (uint8_t), some
>> unnecessary assertion also be dropped.
>>
>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> It looks to me as if the code was more correct without these changes,
> and doing these changes just to work around a pretty obvious
> Coverity flaw seems rather odd.
> 

Okay, then please consider merge 1/3 and 2/3.

-- 
Regards,
-Bob