[PATCH v5 02/15] xen: check parameter validity when parsing command line

[Juergen Gross <jgross@suse.com>]

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@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
V4:
- removed test for empty string from parse_bool() again, avoid calling
  parse_bool() in that case (Jan Beulich)
- print error number in failure case (Jan Beulich)
- limit scope of some variables (Jan Beulich)
- use type bool for bool_assert (Jan Beulich)

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)

V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()
---
 xen/common/kernel.c     | 68 ++++++++++++++++++++++++++++++++++++++-----------
 xen/include/xen/init.h  | 30 +++++++++++++++++-----
 xen/include/xen/types.h |  3 +++
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 71bc547d17..a0611d6c7c 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, *key;
     const struct kernel_param *param;
-    int bool_assert;
+    int rc;
+    bool bool_assert, found;
 
     for ( ; ; )
     {
@@ -84,46 +93,66 @@ 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++ )
         {
+            int rctmp;
+            const char *s;
+
             if ( strcmp(param->name, optkey) )
             {
                 if ( param->type == OPT_CUSTOM && q &&
                      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 ? optval : "yes", NULL) )
+                rctmp = *optval ? parse_bool(optval, NULL) : 0;
+                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 +160,22 @@ 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\", rc=%d!\n",
+                    key, optval, rc);
+        if ( !found )
+            printk("parameter \"%s\" unknown!\n", key);
     }
 }
 
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@lists.xen.org
https://lists.xen.org/xen-devel