| Message ID | becbee16d2ee503a7f698364aef672d4cadf5079.1638907336.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sparse-checkout: fix segfault on malformed patterns | expand |
On Tue, Dec 7, 2021 at 12:02 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > 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@johnburnett.com> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > 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 >
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