libxc: fix leak of t_info in xc_tbuf_get_size()

Avoid leaking the memory mapping of the trace buffer

Coverity ID 1351228

Signed-off-by Harmandeep Kaur <[email protected]>

Avoid leaking the memory mapping of the trace buffer

Coverity ID 1351228

Signed-off-by Harmandeep Kaur <[email protected]>

---
v2: call to unmapping function reduced to one from two
---
 tools/libxc/xc_tbuf.c | 8 +++++---

 1 file changed, 5 insertions(+), 3 deletions(-)

tools/libxc/xc_tbuf.c +0 -0

index 695939a..d96cc67 100644
@@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch, unsigned long
 *size)
                      sysctl.u.tbuf_op.buffer_mfn);

     if ( t_info == NULL || t_info->tbuf_size == 0 ) -        return -1;
+        rc = -1;
+    else

tools/libxc/xc_tbuf.c +1 -0

+	*size = t_info->tbuf_size;

-    *size = t_info->tbuf_size;
+    xenforeignmemory_unmap(xch->fmem, t_info, *size);

-    return 0;
+    return rc;
 }

 int xc_tbuf_enable(xc_interface *xch, unsigned long pages, unsigned long *mfn,

Quoted 16 lines · Harmandeep Kaur

tools/libxc/xc_tbuf.c +0 -0

index 695939a..d96cc67 100644
@@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,

unsigned long
*size)
                     sysctl.u.tbuf_op.buffer_mfn);
     if ( t_info == NULL || t_info->tbuf_size == 0 ) -        return -1;
+        rc = -1;
+    else

tools/libxc/xc_tbuf.c +1 -0

+	*size = t_info->tbuf_size;

Quoted 2 lines

-    *size = t_info->tbuf_size;
+    xenforeignmemory_unmap(xch->fmem, t_info, *size);

*size could be uninitialised here (in the error path) and even in the success case I don't think t_info->tbus_size is the right argument here, it
needs to be the size which was passed to the map function, i.e. sysctl.u.tbuf_op.size.

Quoted 17 lines · Ian Campbell

Quoted 16 lines · Harmandeep Kaur

tools/libxc/xc_tbuf.c +0 -0

index 695939a..d96cc67 100644
@@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,

unsigned long
*size)
                     sysctl.u.tbuf_op.buffer_mfn);
     if ( t_info == NULL || t_info->tbuf_size == 0 ) -        return -1;
+        rc = -1;
+    else

tools/libxc/xc_tbuf.c +1 -0

+	*size = t_info->tbuf_size;

Quoted 9 lines

Quoted 2 lines

-    *size = t_info->tbuf_size;
+    xenforeignmemory_unmap(xch->fmem, t_info, *size);

*size could be uninitialised here (in the error path) and even in the success case I don't think t_info->tbus_size is the right argument here, it
needs to be the size which was passed to the map function, i.e. sysctl.u.tbuf_op.size.

And I think both are issues with the current code,

and, more important,
not what Coverity is complaining about in the referenced CID?

To be clear, I'm not arguing that they're not issues we should fix (I don't know about the tbuf_size vs. tbuf_op.size, but I can check). I'm genuinely asking whether we should do that here, as compared to in a pre or follow up patch, and let this one be the one that placates Coverity on that particular issue.

Quoted 29 lines · Dario Faggioli

Quoted 28 lines · Ian Campbell

Quoted 15 lines · Harmandeep Kaur

tools/libxc/xc_tbuf.c +0 -0

index 695939a..d96cc67 100644
@@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,

unsigned long
*size)
                     sysctl.u.tbuf_op.buffer_mfn);
     if ( t_info == NULL || t_info->tbuf_size == 0 ) -        return -1;
+        rc = -1;
+    else

tools/libxc/xc_tbuf.c +1 -0

+	*size = t_info->tbuf_size;

Quoted 3 lines

-    *size = t_info->tbuf_size;
+    xenforeignmemory_unmap(xch->fmem, t_info, *size);

*size could be uninitialised here (in the error path) and even in the
success case I don't think t_info->tbus_size is the right argument
here, it
needs to be the size which was passed to the map function, i.e. sysctl.u.tbuf_op.size.

Quoted 1 line

And I think both are issues with the current code,

I don't think so, the xenforeignmemory_unmap using *size as an argument
(where it is either uninitialised or the wrong value) is added by this
patch.