All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Paul Durrant <paul@xen.org>, Wei Liu <wl@xen.org>,
	Ian Jackson <Ian.Jackson@citrix.com>
Subject: [PATCH for-4.14] tools/libxc: Drop config_transformed parameter from xc_cpuid_set()
Date: Fri, 12 Jun 2020 11:55:19 +0100	[thread overview]
Message-ID: <20200612105519.18728-1-andrew.cooper3@citrix.com> (raw)

libxl is now the sole caller of xc_cpuid_set().  The config_transformed output
is ignored, and this patch trivially highlights the resulting memory leak.

"transformed" config is now properly forwarded on migrate as part of the
general VM state, so delete the transformation logic completely, rather than
trying to adjust just libxl to avoid leaking memory.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Paul Durrant <paul@xen.org>

For 4.14 for hopefully obvious reasons.

Ian: for backport to 4.13 and earlier, there are a number of options.  The
reasoning we used to delete the other callers of xc_cpuid_set() is still
valid, but probably not backport material.

OTOH, moding libxl_cpuid_set() (as it was back then) to loop over cpuid_res[]
and free them all should work.
---
 tools/libxc/include/xenctrl.h |  3 +--
 tools/libxc/xc_cpuid_x86.c    | 25 +------------------------
 tools/libxl/libxl_cpuid.c     |  3 +--
 3 files changed, 3 insertions(+), 28 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index f9e17ae424..113ddd935d 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1795,8 +1795,7 @@ int xc_domain_debug_control(xc_interface *xch,
 int xc_cpuid_set(xc_interface *xch,
                  uint32_t domid,
                  const unsigned int *input,
-                 const char **config,
-                 char **config_transformed);
+                 const char **config);
 
 /*
  * Make adjustments to the CPUID settings for a domain.
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 89d2ecdad2..b42edd6457 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -279,7 +279,7 @@ int xc_set_domain_cpu_policy(xc_interface *xch, uint32_t domid,
  */
 int xc_cpuid_set(
     xc_interface *xch, uint32_t domid, const unsigned int *input,
-    const char **config, char **config_transformed)
+    const char **config)
 {
     int rc;
     unsigned int i, j, regs[4] = {}, polregs[4] = {};
@@ -288,9 +288,6 @@ int xc_cpuid_set(
     unsigned int nr_leaves, policy_leaves, nr_msrs;
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
 
-    for ( i = 0; i < 4; ++i )
-        config_transformed[i] = NULL;
-
     if ( xc_domain_getinfo(xch, domid, 1, &di) != 1 ||
          di.domid != domid )
     {
@@ -365,13 +362,6 @@ int xc_cpuid_set(
             continue;
         }
 
-        config_transformed[i] = calloc(33, 1); /* 32 bits, NUL terminator. */
-        if ( config_transformed[i] == NULL )
-        {
-            rc = -ENOMEM;
-            goto fail;
-        }
-
         /*
          * Notes for following this algorithm:
          *
@@ -399,11 +389,6 @@ int xc_cpuid_set(
                 set_feature(31 - j, regs[i]);
             else
                 clear_feature(31 - j, regs[i]);
-
-            config_transformed[i][j] = config[i][j];
-            /* All non 0/1 values get overwritten. */
-            if ( (config[i][j] & ~1) != '0' )
-                config_transformed[i][j] = '0' + val;
         }
     }
 
@@ -421,16 +406,8 @@ int xc_cpuid_set(
     }
 
     /* Success! */
-    goto out;
 
  fail:
-    for ( i = 0; i < 4; i++ )
-    {
-        free(config_transformed[i]);
-        config_transformed[i] = NULL;
-    }
-
- out:
     free(leaves);
 
     return rc;
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 4e4852ddeb..796ec4f2d9 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -421,7 +421,6 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid,
 {
     libxl_cpuid_policy_list cpuid = info->cpuid;
     int i;
-    char *cpuid_res[4];
     bool pae = true;
 
     /*
@@ -444,7 +443,7 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid,
 
     for (i = 0; cpuid[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++)
         xc_cpuid_set(ctx->xch, domid, cpuid[i].input,
-                     (const char**)(cpuid[i].policy), cpuid_res);
+                     (const char**)cpuid[i].policy);
 }
 
 static const char *input_names[2] = { "leaf", "subleaf" };
-- 
2.11.0



             reply	other threads:[~2020-06-12 10:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12 10:55 Andrew Cooper [this message]
2020-06-12 11:05 ` [PATCH for-4.14] tools/libxc: Drop config_transformed parameter from xc_cpuid_set() Paul Durrant
2020-06-12 15:53   ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200612105519.18728-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=paul@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.