Re: [PATCH] pathspec: fix segfault in clear_pathspec

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 10, 2017, 12:04 UTC
Message-ID: <CACsJy8D9oXDbOqEfPTuSdm0Phvwo6AxQh_Q5tG4k22_mFBT4OA@mail.gmail.com>
In-Reply-To: <20170407192919.10209-1-bmwill@google.com>

On Sat, Apr 8, 2017 at 2:29 AM, Brandon Williams <bmwill@google.com> wrote:

Show 21 quoted lines

> In 'clear_pathspec()' the incorrect index parameter is used to bound an
> inner-loop which is used to free a 'struct attr_match' value field.
> Using the incorrect index parameter (in addition to being incorrect)
> occasionally causes segmentation faults when attempting to free an
> invalid pointer.  Fix this by using the correct index parameter 'i'.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  pathspec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index 303efda83..69ef86b85 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -724,7 +724,7 @@ void clear_pathspec(struct pathspec *pathspec)
>                 free(pathspec->items[i].match);
>                 free(pathspec->items[i].original);
>
> -               for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
> +               for (j = 0; j < pathspec->items[i].attr_match_nr; j++)

Ouch. Perhaps this is a good time to rename 'j' to something better? attr_idx or attr_index, maybe.

Show 6 quoted lines

>                         free(pathspec->items[i].attr_match[j].value);
>                 free(pathspec->items[i].attr_match);
>
> --
> 2.12.2.715.g7642488e1d-goog
>

-- 
Duy