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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.