custom parameter handling fixes

Commit Message

Jan Beulich Sept. 14, 2017, 8:57 a.m. UTC

The recent changes to their handling introduced a few false warnings,
due to checks looking at the wrong string slot. While going through all
those commits and looking for patterns similar to the "dom0_mem=" I've
noticed this with, I also realized that there were other issues with
"dom0_nodes=" and "rmrr=", partly pre-existing, but partly also due to
those recent changes not having gone far enough.

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Tian, Kevin Sept. 20, 2017, 6:54 a.m. UTC | #1

> From: Jan Beulich [mailto:[e-mail]]
> Sent: Thursday, September 14, 2017 4:57 PM
> 
> The recent changes to their handling introduced a few false warnings,
> due to checks looking at the wrong string slot. While going through all
> those commits and looking for patterns similar to the "dom0_mem=" I've
> noticed this with, I also realized that there were other issues with
> "dom0_nodes=" and "rmrr=", partly pre-existing, but partly also due to
> those recent changes not having gone far enough.
> 
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Kevin Tian <[e-mail]>

Andrew Cooper Sept. 22, 2017, 2:11 p.m. UTC | #2

On 14/09/17 09:57, Jan Beulich wrote:
> The recent changes to their handling introduced a few false warnings,
> due to checks looking at the wrong string slot. While going through all
> those commits and looking for patterns similar to the "dom0_mem=" I've
> noticed this with, I also realized that there were other issues with
> "dom0_nodes=" and "rmrr=", partly pre-existing, but partly also due to
> those recent changes not having gone far enough.
>
> Signed-off-by: Jan Beulich <[e-mail]>

Acked-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -59,7 +59,7 @@  static int __init parse_dom0_mem(const c
             dom0_nrpages = parse_amt(s, &s);
     } while ( *s++ == ',' );
 
-    return *s ? -EINVAL : 0;
+    return s[-1] ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
@@ -94,7 +94,13 @@  static int __init parse_dom0_nodes(const
 {
     do {
         if ( isdigit(*s) )
+        {
+            if ( dom0_nr_pxms >= ARRAY_SIZE(dom0_pxms) )
+                return -E2BIG;
             dom0_pxms[dom0_nr_pxms] = simple_strtoul(s, &s, 0);
+            if ( !*s || *s == ',' )
+                ++dom0_nr_pxms;
+        }
         else if ( !strncmp(s, "relaxed", 7) && (!s[7] || s[7] == ',') )
         {
             dom0_affinity_relaxed = true;
@@ -106,10 +112,10 @@  static int __init parse_dom0_nodes(const
             s += 6;
         }
         else
-            break;
-    } while ( ++dom0_nr_pxms < ARRAY_SIZE(dom0_pxms) && *s++ == ',' );
+            return -EINVAL;
+    } while ( *s++ == ',' );
 
-    return *s ? -EINVAL : 0;
+    return s[-1] ? -EINVAL : 0;
 }
 custom_param("dom0_nodes", parse_dom0_nodes);
 
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -1097,15 +1097,18 @@  static int __init parse_rmrr_param(const
     unsigned long start, end;
 
     do {
+        if ( nr_rmrr >= MAX_USER_RMRR )
+            return -E2BIG;
+
         start = simple_strtoul(cur = s, &s, 16);
         if ( cur == s )
-            break;
+            return -EINVAL;
 
         if ( *s == '-' )
         {
             end = simple_strtoul(cur = s + 1, &s, 16);
             if ( cur == s )
-                break;
+                return -EINVAL;
         }
         else
             end = start;
@@ -1121,7 +1124,7 @@  static int __init parse_rmrr_param(const
 
             stmp = parse_pci_seg(s + 1, &seg, &bus, &dev, &func, &def_seg);
             if ( !stmp )
-                break;
+                return -EINVAL;
 
             /*
              * Not specified.
@@ -1142,8 +1145,8 @@  static int __init parse_rmrr_param(const
         if ( user_rmrrs[nr_rmrr].dev_count )
             nr_rmrr++;
 
-    } while ( *s++ == ';' && nr_rmrr < MAX_USER_RMRR );
+    } while ( *s++ == ';' );
 
-    return *s ? -EINVAL : 0;
+    return s[-1] ? -EINVAL : 0;
 }
 custom_param("rmrr", parse_rmrr_param);