[PATCH] submodule add: sanity check existing .gitmodules
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 16, 2025, 07:02 UTC
- Message-ID
- <xmqqv7jacvdq.fsf@gitster.g>
"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