From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org, K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Subject: [PATCH] submodule add: sanity check existing .gitmodules
Date: Sat, 15 Nov 2025 23:02:57 -0800 [thread overview]
Message-ID: <xmqqv7jacvdq.fsf@gitster.g> (raw)
"git submodule add" tries to find if a submodule with the same name
already exists at a different path, by looking up an entry in the
.gitmodules file. If the entry in the file is incomplete, e.g.,
when the submodule.<name>.something variable is defined but there is
no definition of submodule.<name>.path variable, it accessing the
missing .path member of the submodule structure and triggers a
segfault.
A brief audit was done to make sure that the code does not assume
members other than those that are absolutely certain to exist: a
submodule obtained by submodule_from_name() should have .name
member, while a submodule obtained by submodule_from_path() should
also have .path as well as .name member, and we cannot assume
anything else. Luckily, the module_add() codepath was the only
problematic one. It is fairly recent code that comes from 1fa06ced
(submodule: prevent overwriting .gitmodules on path reuse,
2025-07-24).
A helper used by update_submodule() seems to assume that its call to
submodule_from_path() always yields a submodule object without a
failure, which seems to rely on the caller's making sure it is the
case. Leave an assert() with a NEEDSWORK comment there for future
developers to make sure the assumption actually holds.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 12 ++++++++++--
t/t7400-submodule-basic.sh | 19 +++++++++++++++++++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 07a1935cbe..1a1043cdab 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1913,6 +1913,13 @@ static int determine_submodule_update_strategy(struct repository *r,
const char *val;
int ret;
+ /*
+ * NEEDSWORK: audit and ensure that update_submodule() has right
+ * to assume that submodule_from_path() above will always succeed.
+ */
+ if (!sub)
+ BUG("update_submodule assumes a submodule exists at path (%s)",
+ path);
key = xstrfmt("submodule.%s.update", sub->name);
if (update) {
@@ -3537,14 +3544,15 @@ static int module_add(int argc, const char **argv, const char *prefix,
}
}
- if(!add_data.sm_name)
+ if (!add_data.sm_name)
add_data.sm_name = add_data.sm_path;
existing = submodule_from_name(the_repository,
null_oid(the_hash_algo),
add_data.sm_name);
- if (existing && strcmp(existing->path, add_data.sm_path)) {
+ if (existing && existing->path &&
+ strcmp(existing->path, add_data.sm_path)) {
if (!force) {
die(_("submodule name '%s' already used for path '%s'"),
add_data.sm_name, existing->path);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fd3e7e355e..9ade97e432 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -48,6 +48,25 @@ test_expect_success 'submodule deinit works on empty repository' '
git submodule deinit --all
'
+test_expect_success 'submodule add with incomplete .gitmodules' '
+ test_when_finished "rm -f expect actual" &&
+ test_when_finished "git config remove-section submodule.one" &&
+ test_when_finished "git rm -f one .gitmodules" &&
+ git init one &&
+ git -C one commit --allow-empty -m one-initial &&
+ git config -f .gitmodules submodule.one.ignore all &&
+
+ git submodule add ./one &&
+
+ for var in ignore path url
+ do
+ git config -f .gitmodules --get "submodule.one.$var" ||
+ return 1
+ done >actual &&
+ test_write_lines all one ./one >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'setup - initial commit' '
>t &&
git add t &&
--
2.52.0-rc2-455-g230fcf2819
next reply other threads:[~2025-11-16 7:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-16 7:02 Junio C Hamano [this message]
2025-11-25 6:49 ` [PATCH] submodule add: sanity check existing .gitmodules Elijah Newren
2025-11-25 14:33 ` Junio C Hamano
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=xmqqv7jacvdq.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jayatheerthkulkarni2005@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).