On Tue, Dec 7, 2021 at 12:02 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
> used to populate two hashsets that accelerate pattern matching. If the user
> modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
> then strange patterns can happen, triggering some error checks.
>
> One of these error checks is possible to hit when some special characters
> exist in a line. A warning message is correctly written to stderr, but then
> there is additional logic that attempts to remove the line from the hashset
> and free the data. This leads to a segfault in the 'git sparse-checkout
> list' command because it iterates over the contents of the hashset, which is
> no invalid.
s/no invalid/now invalid/ ?
>
> The fix here is to stop trying to remove from the hashset. Better to leave
> bad data in the sparse-checkout matching logic (with a warning) than to
> segfault. If we are in this state, then we are already traversing into
> undefined behavior, so this change to keep the entry in the hashset is no
> worse than removing it.
>
> Add a test that triggers the segfault without the code change.
>
> Reported-by: John Burnett <johnburnett@xxxxxxxxxxxxxxx>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> dir.c | 3 ---
> t/t1091-sparse-checkout-builtin.sh | 15 +++++++++++++++
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 5aa6fbad0b7..0693c7cb3ee 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -819,9 +819,6 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
> /* we already included this at the parent level */
> warning(_("your sparse-checkout file may have issues: pattern '%s' is repeated"),
> given->pattern);
> - hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
> - free(data);
> - free(translated);
> }
>
> return;
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 272ba1b566b..c72b8ee2e7b 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -708,4 +708,19 @@ test_expect_success 'cone mode clears ignored subdirectories' '
> test_cmp expect out
> '
>
> +test_expect_success 'malformed cone-mode patterns' '
> + git -C repo sparse-checkout init --cone &&
> + mkdir -p repo/foo/bar &&
> + touch repo/foo/bar/x repo/foo/y &&
> + cat >repo/.git/info/sparse-checkout <<-\EOF &&
> + /*
> + !/*/
> + /foo/
> + !/foo/*/
> + /foo/\*/
> + EOF
> + cat repo/.git/info/sparse-checkout &&
> + git -C repo sparse-checkout list
> +'
> +
> test_done
> --
> gitgitgadget
>
