Re: [Minios-devel] [UNIKRAFT PATCH 3/8] lib/uklibparam: Add support for passing array

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Sharan,

as it stands, the array parsing will happily write beyond the bounds of 
the memory area allocated for the array, if you do something like this:
__u16 parmarray[4];
UK_LIB_PARAM_ARR(parmarray, __u16, 6);

My understanding is that the array needs to be statically allocated, so 
you could sizeof the variable passed in and sanity-check it against the 
sizeof the elements and the number of elements. Even better, this could 
even be done at compile time, throwing an error if the two numbers don't 
match. (Side note: we really could use a BUILD_BUG_ON implementation...)
Alternatively, UK_LIB_PARAM_ARR could even be changed to only take two 
parameters, and calculate the number of entries itself, preventing 
mishaps where the two numbers don't match.
And then at run time, libparam can refuse to write more parameters into 
the array than there is space for, and could throw a warning that 
additional array values got ignored.
Cheers,
Florian

On 3/15/19 6:06 PM, Sharan Santhanam wrote:
> This patch implements a way to pass array parameters to the library.
> The array parameters are delimited by a ' '.
>
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>   lib/uklibparam/include/uk/libparam.h | 14 ++++++++++++++
>   lib/uklibparam/param.c               | 27 ++++++++++++++++++++++++++-
>   2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/lib/uklibparam/include/uk/libparam.h 
> b/lib/uklibparam/include/uk/libparam.h
> index 6c3d78a..e23dda4 100644
> --- a/lib/uklibparam/include/uk/libparam.h
> +++ b/lib/uklibparam/include/uk/libparam.h
> @@ -396,6 +396,20 @@ void _uk_libparam_lib_add(struct uk_lib_section *lib_sec);
>         _LIB_UK_PARAM_SET(name, type, 1)                                \
>   
>   /**
> + * Declare an array of primitive.
> + * @param name
> + *     The name of the parameter.
> + * @param type
> + *     The type of the parameter.
> + * @param size
> + *     The number of element of the argument
> + */
> +#define UK_LIB_PARAM_ARR(name, type, size)                             \
> +       _LIB_PARAM_NAME_SET(name, _LIB_PARAM_STRING(UK_LIBPARAM_PREFIX, \
> +                                                   name));             \
> +       _LIB_UK_PARAM_SET(name, type, size)                             \
> +
> +/**
>    * Declare a string library param.
>    * @param name
>    *    The name of the parameter.
> diff --git a/lib/uklibparam/param.c b/lib/uklibparam/param.c
> index eb263be..4dd3b34 100644
> --- a/lib/uklibparam/param.c
> +++ b/lib/uklibparam/param.c
> @@ -40,6 +40,7 @@
>   #include <uk/assert.h>
>   #include <uk/libparam.h>
>   
> +#define ARRAY_SEP	 ' '
>   #define LIB_ARG_SEP    "--"
>   
>   struct param_args {
> @@ -313,6 +314,8 @@ static int kernel_args_set(struct param_args *pargs,
>                            struct uk_param *param)
>   {
>         int rc = 0;
> +       int i  = 0;
> +       char *start, *value;
>         int sign = (param->param_type >> PARAM_SIGN_SHIFT) & PARAM_SIGN_MASK;
>         int scopy = (param->param_type >> PARAM_SCOPY_SHIFT) & PARAM_SCOPY_MASK;
>         int param_type = (param->param_type >> PARAM_SIZE_SHIFT)
> @@ -322,7 +325,29 @@ static int kernel_args_set(struct param_args *pargs,
>                 /* Reference the pointer instead of copying the value */
>                 *((__uptr *)param->addr) = (__uptr) pargs->value;
>         else {
> -               if (param->param_size == 1) {
> +               if (param->param_size > 1) {
> +                       /* Adding support for array */
> +                       i = 0;
> +                       value = &pargs->value[i];
> +                       uk_pr_debug("%s (%d) args in array\n", value,
> +                                    pargs->value_len);
> +                       while (value && i < param->param_size) {
> +                               start = value;
> +                               value = strchr(value, ARRAY_SEP);
> +                               if (value) {
> +                                       uk_pr_debug("Delimiter: %p\n", value);
> +                                       *value = '\0';
> +                                       /* Search from the next index */
> +                                       value++;
> +                               }
> +                               uk_pr_debug("Array index: %d %s\n",
> +                                           i, start);
> +                               rc = kernel_arg_set((void *)(param->addr +
> +                                                   (i * param_type)),
> +                                                   start, param_type, sign);
> +                               i++;
> +                       }
> +               } else if (param->param_size == 1) {
>                         rc = kernel_arg_set((void *)param->addr,
>                                             pargs->value, param_type, sign);
>                 } else {
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel