Re: [PATCH 3/3] libmutipath: validate the argument count of config strings

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

 



On Tue, 2022-12-13 at 17:36 -0600, Benjamin Marzinski wrote:
> The features, path_selector, and hardware_handler config options pass
> their strings directly into the kernel.  If users omit the argument
> counts from these strings, or use the wrong value, the kernel's table
> parsing gets completely messed up, and the error messages it prints
> don't reflect what actully went wrong. To avoid messing up the
> kernel table parsing, verify that these strings correctly set the
> argument count to the number of arguments they have.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
>  libmultipath/dict.c | 110 ++++++++++++++++++++++++++++++++++++++++--
> --
>  1 file changed, 101 insertions(+), 9 deletions(-)
> 
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index f4233882..6645de49 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -116,6 +116,58 @@ set_str(vector strvec, void *ptr, const char
> *file, int line_nr)
>         return 0;
>  }
>  
> +static int
> +set_arg_str(vector strvec, void *ptr, int count_idx, const char
> *file,
> +           int line_nr)
> +{
> +       char **str_ptr = (char **)ptr;
> +       char *old_str = *str_ptr;
> +       const char *spaces = " \f\n\r\t\v";

Nit: I believe '\n' can't occur in values passed from multipath.conf,
as we don't support multi-line values. Also, should this be "static
const char * const spaces", maybe?

Other than that, this looks good to me.

Regards,
Martin



> +       char *p, *end;
> +       int idx = -1;
> +       long int count = -1;
> +
> +       *str_ptr = set_value(strvec);
> +       if (!*str_ptr) {
> +               free(old_str);
> +               return 1;
> +       }
> +       p = *str_ptr;
> +       while (*p != '\0') {
> +               p += strspn(p, spaces);
> +               if (*p == '\0')
> +                       break;
> +               idx += 1;
> +               if (idx == count_idx) {
> +                       errno = 0;
> +                       count = strtol(p, &end, 10);
> +                       if (errno == ERANGE || end == p ||
> +                           !(isspace(*end) || *end == '\0')) {
> +                               count = -1;
> +                               break;
> +                       }
> +               }
> +               p += strcspn(p, spaces);
> +       }
> +       if (count < 0) {
> +               condlog(1, "%s line %d, missing argument count for
> %s",
> +                       file, line_nr, (char*)VECTOR_SLOT(strvec,
> 0));
> +               goto fail;
> +       }
> +       if (count != idx - count_idx) {
> +               condlog(1, "%s line %d, invalid argument count for
> %s:, got '%ld' expected '%d'",
> +                       file, line_nr, (char*)VECTOR_SLOT(strvec, 0),
> count,
> +                       idx - count_idx);
> +               goto fail;
> +       }
> +       free(old_str);
> +       return 0;
> +fail:
> +       free(*str_ptr);
> +       *str_ptr = old_str;
> +       return 0;
> +}
> +
>  static int
>  set_path(vector strvec, void *ptr, const char *file, int line_nr)
>  {
> @@ -288,6 +340,14 @@ def_ ## option ## _handler (struct config *conf,
> vector strvec,         \
>         return set_int(strvec, &conf->option, minval, maxval, file,
> line_nr); \
>  }
>  
> +#define declare_def_arg_str_handler(option,
> count_idx)                 \
> +static
> int                                                             \
> +def_ ## option ## _handler (struct config *conf, vector
> strvec,                \
> +                           const char *file, int
> line_nr)              \
> +{                                                                   
>    \
> +       return set_arg_str(strvec, &conf->option, count_idx, file,
> line_nr); \
> +}
> +
>  #define declare_def_snprint(option,
> function)                          \
>  static
> int                                                             \
>  snprint_def_ ## option (struct config *conf, struct strbuf
> *buff,      \
> @@ -340,6 +400,17 @@ hw_ ## option ## _handler (struct config *conf,
> vector strvec,             \
>         return set_int(strvec, &hwe->option, minval, maxval, file,
> line_nr); \
>  }
>  
> +#define declare_hw_arg_str_handler(option,
> count_idx)                  \
> +static
> int                                                             \
> +hw_ ## option ## _handler (struct config *conf, vector
> strvec,         \
> +                           const char *file, int
> line_nr)              \
> +{                                                                   
>    \
> +       struct hwentry * hwe = VECTOR_LAST_SLOT(conf-
> >hwtable);         \
> +       if
> (!hwe)                                                       \
> +               return
> 1;                                               \
> +       return set_arg_str(strvec, &hwe->option, count_idx, file,
> line_nr); \
> +}
> +
>  
>  #define declare_hw_snprint(option,
> function)                           \
>  static
> int                                                             \
> @@ -371,6 +442,16 @@ ovr_ ## option ## _handler (struct config *conf,
> vector strvec,            \
>                        file, line_nr); \
>  }
>  
> +#define declare_ovr_arg_str_handler(option,
> count_idx)                 \
> +static
> int                                                             \
> +ovr_ ## option ## _handler (struct config *conf, vector
> strvec,                \
> +                           const char *file, int
> line_nr)              \
> +{                                                                   
>    \
> +       if (!conf-
> >overrides)                                           \
> +               return
> 1;                                               \
> +       return set_arg_str(strvec, &conf->overrides->option,
> count_idx, file, line_nr); \
> +}
> +
>  #define declare_ovr_snprint(option,
> function)                          \
>  static
> int                                                             \
>  snprint_ovr_ ## option (struct config *conf, struct strbuf
> *buff,      \
> @@ -401,6 +482,17 @@ mp_ ## option ## _handler (struct config *conf,
> vector strvec,             \
>         return set_int(strvec, &mpe->option, minval, maxval, file,
> line_nr); \
>  }
>  
> +#define declare_mp_arg_str_handler(option,
> count_idx)                  \
> +static
> int                                                             \
> +mp_ ## option ## _handler (struct config *conf, vector
> strvec,         \
> +                           const char *file, int
> line_nr)              \
> +{                                                                   
>    \
> +       struct mpentry * mpe = VECTOR_LAST_SLOT(conf-
> >mptable);         \
> +       if
> (!mpe)                                                       \
> +               return
> 1;                                               \
> +       return set_arg_str(strvec, &mpe->option, count_idx, file,
> line_nr); \
> +}
> +
>  #define declare_mp_snprint(option,
> function)                           \
>  static
> int                                                             \
>  snprint_mp_ ## option (struct config *conf, struct strbuf
> *buff,       \
> @@ -584,13 +676,13 @@ snprint_def_marginal_pathgroups(struct config
> *conf, struct strbuf *buff,
>  }
>  
>  
> -declare_def_handler(selector, set_str)
> +declare_def_arg_str_handler(selector, 1)
>  declare_def_snprint_defstr(selector, print_str, DEFAULT_SELECTOR)
> -declare_hw_handler(selector, set_str)
> +declare_hw_arg_str_handler(selector, 1)
>  declare_hw_snprint(selector, print_str)
> -declare_ovr_handler(selector, set_str)
> +declare_ovr_arg_str_handler(selector, 1)
>  declare_ovr_snprint(selector, print_str)
> -declare_mp_handler(selector, set_str)
> +declare_mp_arg_str_handler(selector, 1)
>  declare_mp_snprint(selector, print_str)
>  
>  static int snprint_uid_attrs(struct config *conf, struct strbuf
> *buff,
> @@ -663,13 +755,13 @@ declare_hw_snprint(prio_args, print_str)
>  declare_mp_handler(prio_args, set_str)
>  declare_mp_snprint(prio_args, print_str)
>  
> -declare_def_handler(features, set_str)
> +declare_def_arg_str_handler(features, 0)
>  declare_def_snprint_defstr(features, print_str, DEFAULT_FEATURES)
> -declare_ovr_handler(features, set_str)
> +declare_ovr_arg_str_handler(features, 0)
>  declare_ovr_snprint(features, print_str)
> -declare_hw_handler(features, set_str)
> +declare_hw_arg_str_handler(features, 0)
>  declare_hw_snprint(features, print_str)
> -declare_mp_handler(features, set_str)
> +declare_mp_arg_str_handler(features, 0)
>  declare_mp_snprint(features, print_str)
>  
>  declare_def_handler(checker_name, set_str)
> @@ -1821,7 +1913,7 @@ declare_hw_snprint(revision, print_str)
>  declare_hw_handler(bl_product, set_str)
>  declare_hw_snprint(bl_product, print_str)
>  
> -declare_hw_handler(hwhandler, set_str)
> +declare_hw_arg_str_handler(hwhandler, 0)
>  declare_hw_snprint(hwhandler, print_str)
>  
>  /*



--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux