From 3dfd30598bdb34078329aa9bd9b03c5ab5cdbcce Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 17 Mar 2021 15:30:48 +0000 Subject: [PATCH 1/2] fsmonitor: fix memory corruption in some corner cases In 56c6910028a (fsmonitor: change last update timestamp on the index_state to opaque token, 2020-01-07), we forgot to adjust the part of `unpack_trees()` that copies the FSMonitor "last-update" information that we copy from the source index to the result index since 679f2f9fdd2 (unpack-trees: skip stat on fsmonitor-valid files, 2019-11-20). Since the "last-update" information is no longer a 64-bit number, but a free-form string that has been allocated, we need to duplicate it rather than just copying it. This is important because there _are_ cases when `unpack_trees()` will perform a oneway merge that implicitly calls `refresh_fsmonitor()` (which will allocate that "last-update" token). This happens _after_ that token was copied into the result index. However, we _then_ call `check_updates()` on that index, which will _also_ call `refresh_fsmonitor()`, accessing the "last-update" string, which by now would be released already. In the instance that lead to this patch, this caused a segmentation fault during a lengthy, complicated rebase involving the todo command `reset` that (crucially) had to updated many files. Unfortunately, it seems very hard to trigger that crash, therefore this patch is not accompanied by a regression test. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- unpack-trees.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 2344b5e6dd..418cf9a29c 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1614,8 +1614,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->merge_size = len; mark_all_ce_unused(o->src_index); - if (o->src_index->fsmonitor_last_update) - o->result.fsmonitor_last_update = o->src_index->fsmonitor_last_update; + o->result.fsmonitor_last_update = + xstrdup_or_null(o->src_index->fsmonitor_last_update); /* * Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries From 4abc57848d8ace8218952c7376fa397c0850392c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 17 Mar 2021 15:30:49 +0000 Subject: [PATCH 2/2] fsmonitor: do not forget to release the token in `discard_index()` In 56c6910028a (fsmonitor: change last update timestamp on the index_state to opaque token, 2020-01-07), we forgot to adjust `discard_index()` to release the "last-update" token: it is no longer a 64-bit number, but a free-form string that has been allocated. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- read-cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/read-cache.c b/read-cache.c index 29144cf879..d19f506cd4 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2355,6 +2355,7 @@ int discard_index(struct index_state *istate) cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; istate->fsmonitor_has_run_once = 0; + FREE_AND_NULL(istate->fsmonitor_last_update); FREE_AND_NULL(istate->cache); istate->cache_alloc = 0; discard_split_index(istate);