Re: [PATCH v2] sparse index: fix use-after-free bug in cache_tree_verify()

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

 



Hi Stolee

On 07/10/2021 16:44, Derrick Stolee wrote:
On 10/7/2021 11:05 AM, Phillip Wood wrote:
Hi Stolee

On 07/10/2021 14:53, Derrick Stolee wrote:
On 10/7/2021 5:50 AM, Phillip Wood via GitGitGadget wrote:
From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
...
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 886e78715fe..85d5279b33c 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -484,7 +484,7 @@ test_expect_success 'checkout and reset (mixed) [sparse]' '
   test_expect_success 'merge, cherry-pick, and rebase' '
       init_repos &&
   -    for OPERATION in "merge -m merge" cherry-pick rebase
+    for OPERATION in "merge -m merge" cherry-pick "rebase --apply" "rebase --merge"

I spoke too soon. On my machine, the 'git rebase --apply' tests fail
because of some verbose output that does not match across the full
and sparse cases. Using "rebase -q --apply" works for me.

Oh, that's strange, the CI tests pass on gitgitgadget and that script passes locally for me. Do you know what the output is that does not match?

It's entirely possible that it's something in git-for-windows/git or
microsoft/git that is causing the difference:

Yes, if I apply the hunk below from 044d9fdaeb ("sparse-checkout:
avoid writing entries with the skip-worktree bit", 2017-03-01) which
is in microsoft/vfs-2.33.0 to my fix then I see the same failure. It
looks like this change makes the apply back end fall back to a three
way merge where a simple patch application succeeded before. Adding
"-q" to the test feels like a bit of a hack but it's probably the best
we can do - at least it still catches any crashes.

Best Wishes

Phillip

diff --git a/apply.c b/apply.c
index 43a0aebf4e..4c1ca6d360 100644
--- a/apply.c
+++ b/apply.c
@@ -3346,6 +3345,24 @@ static int checkout_target(struct index_state *istate,
 {
        struct checkout costate = CHECKOUT_INIT;
+ /*
+        * Do not checkout the entry if the skipworktree bit is set
+        *
+        * Both callers of this method (check_preimage and load_current)
+        * check for the existance of the file before calling this
+        * method so we know that the file doesn't exist at this point
+        * and we don't need to perform that check again here.
+        * We just need to check the skip-worktree and return.
+        *
+        * This is to prevent git from creating a file in the
+        * working directory that has the skip-worktree bit on,
+        * then updating the index from the patch and not keeping
+        * the working directory version up to date with what it
+        * changed the index version to be.
+        */
+       if (ce_skip_worktree(ce))
+               return 0;
+
        costate.refresh_cache = 1;
        costate.istate = istate;
        if (checkout_entry(ce, &costate, NULL, NULL) ||


+ diff -u full-checkout-out sparse-checkout-out
--- full-checkout-out	2021-10-07 13:37:00.475394970 +0000
+++ sparse-checkout-out	2021-10-07 13:37:00.531396095 +0000
@@ -1,3 +1,10 @@
  First, rewinding head to replay your work on top of it...
  Applying: update folder1
+Using index info to reconstruct a base tree...
+Falling back to patching base and 3-way merge...
+Merging:
+e1886b3 update folder2
+virtual update folder1
+found 1 common ancestor:
+virtual b4ad7e16921c16e36f1d5d45ea4fa186efa8422a
  Applying: update deep
+ return 1
error: last command exited with $?=1

[1] https://github.com/microsoft/git/runs/3827705316?check_suite_focus=true#step:5:10469

Thanks,
-Stolee





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux