From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: Derrick Stolee <derrickstolee@github•com>,
Elijah Newren <newren@gmail•com>,
Elijah Newren <newren@gmail•com>,
Elijah Newren <newren@gmail•com>
Subject: [PATCH v2] merge-ort: fix calling merge_finalize() with no intermediate merge
Date: Sat, 22 Apr 2023 20:22:10 +0000 [thread overview]
Message-ID: <pull.1518.v2.git.1682194930766.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1518.git.1681974847078.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail•com>
If some code sets up the data structures for a merge, but then never
actually performs one before calling merge_finalize(), then
merge_finalize() wouldn't notice that result->priv was NULL and
return early, resulting in following that NULL pointer and getting
a segfault. There is currently no code in the git codebase that does
this, but this issue was found during testing of some proposed patches
that had the following structure:
struct merge_options merge_opt;
struct merge_result result;
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
<do N merges, for some value of N>
merge_finalize(&merge_opt, &result);
where some flags could cause the code to have N=0, i.e. doing no merges.
Add a check for result->priv being NULL and return early to avoid a
segfault in these kinds of cases.
While at it, ensure the FREE_AND_NULL() in the function does something
useful with the nulling aspect, namely sets result->priv to NULL rather
than a mere temporary.
Reported-by: Derrick Stolee <derrickstolee@github•com>
Signed-off-by: Elijah Newren <newren@gmail•com>
---
merge-ort: fix calling merge_finalize() with no intermediate merge
Changes since v1:
* Moved code into an if-block instead of returning early, as suggested
by Stolee.
See
https://lore.kernel.org/git/CABPp-BHCdjOutYqdMO1NbYKNA0BgkXRgwUEKK=MX0kXM-5G_DQ@mail.gmail.com/
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1518%2Fnewren%2Ffix-merge-finalize-with-no-merge-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1518/newren/fix-merge-finalize-with-no-merge-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1518
Range-diff vs v1:
1: e636027f60c ! 1: 2cb1c63f05b merge-ort: fix calling merge_finalize() with no intermediate merge
@@ merge-ort.c: void merge_switch_to_result(struct merge_options *opt,
- clear_or_reinit_internal_opts(opti, 0);
- FREE_AND_NULL(opti);
-+ if (!result->priv)
-+ return;
-+ clear_or_reinit_internal_opts(result->priv, 0);
-+ FREE_AND_NULL(result->priv);
++ if (result->priv) {
++ clear_or_reinit_internal_opts(result->priv, 0);
++ FREE_AND_NULL(result->priv);
++ }
}
/*** Function Grouping: helper functions for merge_incore_*() ***/
merge-ort.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index 5bf64354d16..29966fc082f 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4718,14 +4718,14 @@ void merge_switch_to_result(struct merge_options *opt,
void merge_finalize(struct merge_options *opt,
struct merge_result *result)
{
- struct merge_options_internal *opti = result->priv;
-
if (opt->renormalize)
git_attr_set_direction(GIT_ATTR_CHECKIN);
assert(opt->priv == NULL);
- clear_or_reinit_internal_opts(opti, 0);
- FREE_AND_NULL(opti);
+ if (result->priv) {
+ clear_or_reinit_internal_opts(result->priv, 0);
+ FREE_AND_NULL(result->priv);
+ }
}
/*** Function Grouping: helper functions for merge_incore_*() ***/
base-commit: 667fcf4e15379790f0b609d6a83d578e69f20301
--
gitgitgadget
next prev parent reply other threads:[~2023-04-22 20:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-20 7:14 [PATCH] merge-ort: fix calling merge_finalize() with no intermediate merge Elijah Newren via GitGitGadget
2023-04-20 13:10 ` Derrick Stolee
2023-04-20 17:10 ` Junio C Hamano
2023-04-22 2:04 ` Elijah Newren
2023-04-20 16:11 ` Junio C Hamano
2023-04-22 20:22 ` Elijah Newren via GitGitGadget [this message]
2023-04-24 15:17 ` [PATCH v2] " Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1518.v2.git.1682194930766.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail$(echo .)com \
--cc=derrickstolee@github$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=newren@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox