[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v3 39/52] xen: check parameter validity when parsing command line



Where possible check validity of parameters in _cmdline_parse() and
issue a warning message in case of an error detected.

In order to make sure a custom parameter parsing function really
returns a value (error or success), don't use a void pointer for
storing the function address, but a proper typed function pointer.

Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()

V3:
- use function pointer in struct kernel_param (Jan Beulich)
- better range check in assign_integer_param() (Jan Beulich)
- dont assign int values in case of overflow (Jan Beulich)
- allow multiple handlers for a parameter (Jan Beulich)
---
 xen/common/kernel.c     | 67 +++++++++++++++++++++++++++++++++++++------------
 xen/include/xen/init.h  | 30 +++++++++++++++++-----
 xen/include/xen/types.h |  3 +++
 3 files changed, 78 insertions(+), 22 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ce7cb8adb5..c629ffa11c 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,34 +23,43 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static void __init assign_integer_param(
+static int __init assign_integer_param(
     const struct kernel_param *param, uint64_t val)
 {
     switch ( param->len )
     {
     case sizeof(uint8_t):
-        *(uint8_t *)param->var = val;
+        if ( val > UINT8_MAX && val < (uint64_t)INT8_MIN )
+            return -EOVERFLOW;
+        *(uint8_t *)param->par.var = val;
         break;
     case sizeof(uint16_t):
-        *(uint16_t *)param->var = val;
+        if ( val > UINT16_MAX && val < (uint64_t)INT16_MIN )
+            return -EOVERFLOW;
+        *(uint16_t *)param->par.var = val;
         break;
     case sizeof(uint32_t):
-        *(uint32_t *)param->var = val;
+        if ( val > UINT32_MAX && val < (uint64_t)INT32_MIN )
+            return -EOVERFLOW;
+        *(uint32_t *)param->par.var = val;
         break;
     case sizeof(uint64_t):
-        *(uint64_t *)param->var = val;
+        *(uint64_t *)param->par.var = val;
         break;
     default:
         BUG();
     }
+
+    return 0;
 }
 
 static void __init _cmdline_parse(const char *cmdline)
 {
     char opt[128], *optval, *optkey, *q;
-    const char *p = cmdline;
+    const char *p = cmdline, *s, *key;
     const struct kernel_param *param;
-    int bool_assert;
+    int bool_assert, rctmp, rc;
+    bool found;
 
     for ( ; ; )
     {
@@ -84,10 +93,13 @@ static void __init _cmdline_parse(const char *cmdline)
         }
 
         /* Boolean parameters can be inverted with 'no-' prefix. */
+        key = optkey;
         bool_assert = !!strncmp("no-", optkey, 3);
         if ( !bool_assert )
             optkey += 3;
 
+        rc = 0;
+        found = false;
         for ( param = __setup_start; param < __setup_end; param++ )
         {
             if ( strcmp(param->name, optkey) )
@@ -96,34 +108,48 @@ static void __init _cmdline_parse(const char *cmdline)
                      strlen(param->name) == q + 1 - opt &&
                      !strncmp(param->name, opt, q + 1 - opt) )
                 {
+                    found = true;
                     optval[-1] = '=';
-                    ((void (*)(const char *))param->var)(q);
+                    rctmp = param->par.func(q);
                     optval[-1] = '\0';
+                    if ( !rc )
+                        rc = rctmp;
                 }
                 continue;
             }
 
+            rctmp = 0;
+            found = true;
             switch ( param->type )
             {
             case OPT_STR:
-                strlcpy(param->var, optval, param->len);
+                strlcpy(param->par.var, optval, param->len);
                 break;
             case OPT_UINT:
-                assign_integer_param(
+                rctmp = assign_integer_param(
                     param,
-                    simple_strtoll(optval, NULL, 0));
+                    simple_strtoll(optval, &s, 0));
+                if ( *s )
+                    rctmp = -EINVAL;
                 break;
             case OPT_BOOL:
-                if ( !parse_bool(optval) )
+                rctmp = parse_bool(optval);
+                if ( rctmp < 0 )
+                    break;
+                if ( !rctmp )
                     bool_assert = !bool_assert;
+                rctmp = 0;
                 assign_integer_param(param, bool_assert);
                 break;
             case OPT_SIZE:
-                assign_integer_param(
+                rctmp = assign_integer_param(
                     param,
-                    parse_size_and_unit(optval, NULL));
+                    parse_size_and_unit(optval, &s));
+                if ( *s )
+                    rctmp = -EINVAL;
                 break;
             case OPT_CUSTOM:
+                rctmp = -EINVAL;
                 if ( !bool_assert )
                 {
                     if ( *optval )
@@ -131,13 +157,21 @@ static void __init _cmdline_parse(const char *cmdline)
                     safe_strcpy(opt, "no");
                     optval = opt;
                 }
-                ((void (*)(const char *))param->var)(optval);
+                rctmp = param->par.func(optval);
                 break;
             default:
                 BUG();
                 break;
             }
+
+            if ( !rc )
+                rc = rctmp;
         }
+
+        if ( rc )
+            printk("parameter \"%s\" has invalid value \"%s\"!\n", key, 
optval);
+        if ( !found )
+            printk("parameter \"%s\" unknown!\n", key);
     }
 }
 
@@ -176,7 +210,8 @@ int __init parse_bool(const char *s)
          !strcmp("on", s) ||
          !strcmp("true", s) ||
          !strcmp("enable", s) ||
-         !strcmp("1", s) )
+         !strcmp("1", s) ||
+         !*s )
         return 1;
 
     return -1;
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 25d2eef8dd..234ec25aae 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -83,7 +83,10 @@ struct kernel_param {
         OPT_CUSTOM
     } type;
     unsigned int len;
-    void *var;
+    union {
+        void *var;
+        int (*func)(const char *);
+    } par;
 };
 
 extern const struct kernel_param __setup_start[], __setup_end[];
@@ -95,23 +98,38 @@ extern const struct kernel_param __setup_start[], 
__setup_end[];
 
 #define custom_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
-    __kparam __setup_##_var = { __setup_str_##_var, OPT_CUSTOM, 0, _var }
+    __kparam __setup_##_var = \
+        { .name = __setup_str_##_var, \
+          .type = OPT_CUSTOM, \
+          .par.func = _var }
 #define boolean_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
     __kparam __setup_##_var = \
-        { __setup_str_##_var, OPT_BOOL, sizeof(_var), &_var }
+        { .name = __setup_str_##_var, \
+          .type = OPT_BOOL, \
+          .len = sizeof(_var), \
+          .par.var = &_var }
 #define integer_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
     __kparam __setup_##_var = \
-        { __setup_str_##_var, OPT_UINT, sizeof(_var), &_var }
+        { .name = __setup_str_##_var, \
+          .type = OPT_UINT, \
+          .len = sizeof(_var), \
+          .par.var = &_var }
 #define size_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
     __kparam __setup_##_var = \
-        { __setup_str_##_var, OPT_SIZE, sizeof(_var), &_var }
+        { .name = __setup_str_##_var, \
+          .type = OPT_SIZE, \
+          .len = sizeof(_var), \
+          .par.var = &_var }
 #define string_param(_name, _var) \
     __setup_str __setup_str_##_var[] = _name; \
     __kparam __setup_##_var = \
-        { __setup_str_##_var, OPT_STR, sizeof(_var), &_var }
+        { .name = __setup_str_##_var, \
+          .type = OPT_STR, \
+          .len = sizeof(_var), \
+          .par.var = &_var }
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index b1dbb8720a..03f0fe612e 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -14,12 +14,15 @@
 #define NULL ((void*)0)
 #endif
 
+#define INT8_MIN        (-127-1)
 #define INT16_MIN       (-32767-1)
 #define INT32_MIN       (-2147483647-1)
 
+#define INT8_MAX        (127)
 #define INT16_MAX       (32767)
 #define INT32_MAX       (2147483647)
 
+#define UINT8_MAX       (255)
 #define UINT16_MAX      (65535)
 #define UINT32_MAX      (4294967295U)
 
-- 
2.12.3


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.