Patchwork [06/10] vl.c: handle invalid NUMA CPU ranges properly

login
register
mail settings
Submitter Eduardo Habkost
Date Jan. 11, 2013, 6:15 p.m.
Message ID <1357928108-21066-7-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/211394/
State New
Headers show

Comments

Eduardo Habkost - Jan. 11, 2013, 6:15 p.m.
Add checks for the following cases:

* Empty string: will be ignored and won't set any CPU bitmap,
  parser won't abort.
* Missing end value after "-": parser will abort.
* Extra characters after a valid CPU range: parser will abort.
* "N-M" string where M < N: parser will abort.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
Eric Blake - Jan. 11, 2013, 9:32 p.m.
On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
> Add checks for the following cases:
> 
> * Empty string: will be ignored and won't set any CPU bitmap,
>   parser won't abort.
> * Missing end value after "-": parser will abort.
> * Extra characters after a valid CPU range: parser will abort.
> * "N-M" string where M < N: parser will abort.


>      value = strtoull(cpus, &endptr, 10);
>      if (*endptr == '-') {
> -        endvalue = strtoull(endptr+1, &endptr, 10);
> +        endptr++;
> +        if (!*endptr) {
> +            goto error;
> +        }
> +        endvalue = strtoull(endptr, &endptr, 10);
>      } else {
>          endvalue = value;
>      }

Still missing a check for '-numa=-2' with no number on the left of '-',
as well as missing a check for overflow for -numa=999999999999999999999999
Eduardo Habkost - Jan. 14, 2013, 1:30 p.m.
On Fri, Jan 11, 2013 at 02:32:48PM -0700, Eric Blake wrote:
> On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
> > Add checks for the following cases:
> > 
> > * Empty string: will be ignored and won't set any CPU bitmap,
> >   parser won't abort.
> > * Missing end value after "-": parser will abort.
> > * Extra characters after a valid CPU range: parser will abort.
> > * "N-M" string where M < N: parser will abort.
> 
> 
> >      value = strtoull(cpus, &endptr, 10);
> >      if (*endptr == '-') {
> > -        endvalue = strtoull(endptr+1, &endptr, 10);
> > +        endptr++;
> > +        if (!*endptr) {
> > +            goto error;
> > +        }
> > +        endvalue = strtoull(endptr, &endptr, 10);
> >      } else {
> >          endvalue = value;
> >      }
> 
> Still missing a check for '-numa=-2' with no number on the left of '-',
> as well as missing a check for overflow for -numa=999999999999999999999999

Thanks!

I will fix and submit v2 of this patch.

Patch

diff --git a/vl.c b/vl.c
index 03a826e..19010fa 100644
--- a/vl.c
+++ b/vl.c
@@ -1057,13 +1057,30 @@  static void numa_node_parse_cpus(int nodenr, const char *cpus)
     char *endptr;
     unsigned long long value, endvalue;
 
+    /* Empty strings will be ignored, and not considered an error */
+    if (!*cpus) {
+        return;
+    }
+
     value = strtoull(cpus, &endptr, 10);
     if (*endptr == '-') {
-        endvalue = strtoull(endptr+1, &endptr, 10);
+        endptr++;
+        if (!*endptr) {
+            goto error;
+        }
+        endvalue = strtoull(endptr, &endptr, 10);
     } else {
         endvalue = value;
     }
 
+    if (*endptr != '\0')  {
+        goto error;
+    }
+
+    if (endvalue < value) {
+        goto error;
+    }
+
     if (!(endvalue < MAX_CPUMASK_BITS)) {
         endvalue = MAX_CPUMASK_BITS - 1;
         fprintf(stderr, "A max of %d CPUs are supported in a guest\n",
@@ -1071,6 +1088,11 @@  static void numa_node_parse_cpus(int nodenr, const char *cpus)
     }
 
     bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+    return;
+
+error:
+    fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
+    exit(1);
 }
 
 static void numa_node_add(const char *optarg)