Does it make sense to also add regtest on that regression?

On Mon, Jun 24, 2019, 1:14 AM Tim Duesterhus <t...@bastelstu.be> wrote:

> Consider this configuration:
>
>     frontend fe_http
>         mode http
>         bind *:8080
>
>         default_backend be_http
>
>     backend be_http
>         mode http
>         server example example.com:80
>
>     program foo bar
>
> Running with valgrind results in:
>
>     ==16252== Invalid read of size 8
>     ==16252==    at 0x52AE3F: cfg_parse_program (mworker-prog.c:233)
>     ==16252==    by 0x4823B3: readcfgfile (cfgparse.c:2180)
>     ==16252==    by 0x47BCED: init (haproxy.c:1649)
>     ==16252==    by 0x404E22: main (haproxy.c:2714)
>     ==16252==  Address 0x48 is not stack'd, malloc'd or (recently) free'd
>
> Check whether `ext_child` is valid before attempting to free it and its
> contents.
>
> This bug was introduced in 9a1ee7ac31c56fd7d881adf2ef4659f336e50c9f.
> This fix must be backported to HAProxy 2.0.
> ---
>  src/mworker-prog.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/src/mworker-prog.c b/src/mworker-prog.c
> index 467ce9b24..ba52406e9 100644
> --- a/src/mworker-prog.c
> +++ b/src/mworker-prog.c
> @@ -230,22 +230,24 @@ int cfg_parse_program(const char *file, int linenum,
> char **args, int kwm)
>         return err_code;
>
>  error:
> -       LIST_DEL(&ext_child->list);
> -       if (ext_child->command) {
> -               int i;
> -
> -               for (i = 0; ext_child->command[i]; i++) {
> -                       if (ext_child->command[i]) {
> -                               free(ext_child->command[i]);
> -                               ext_child->command[i] = NULL;
> +       if (ext_child) {
> +               LIST_DEL(&ext_child->list);
> +               if (ext_child->command) {
> +                       int i;
> +
> +                       for (i = 0; ext_child->command[i]; i++) {
> +                               if (ext_child->command[i]) {
> +                                       free(ext_child->command[i]);
> +                                       ext_child->command[i] = NULL;
> +                               }
>                         }
> +                       free(ext_child->command);
> +                       ext_child->command = NULL;
> +               }
> +               if (ext_child->id) {
> +                       free(ext_child->id);
> +                       ext_child->id = NULL;
>                 }
> -               free(ext_child->command);
> -               ext_child->command = NULL;
> -       }
> -       if (ext_child->id) {
> -               free(ext_child->id);
> -               ext_child->id = NULL;
>         }
>
>         free(ext_child);
> --
> 2.21.0
>
>
>

Reply via email to