bug: conflicting core.bare setting causes segfault during bare clone

Thank you for filling out a Git bug report! Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

$ env | grep GIT

$ git config --list user.email=jvusich@amazon.com user.name=Joseph Vusich core.bare=false

$ git clone --bare https://github.com/josephvusich/fixture Cloning into bare repository 'fixture.git'... zsh: segmentation fault /opt/local/bin/git clone --bare https://github.com/josephvusich/fixture

What did you expect to happen? (Expected behavior)

"git clone --bare" should clone a bare repository, regardless of the core.bare setting in the global config

What happened instead? (Actual behavior)

"git clone --bare" causes a segfault if the global gitconfig has core.bare=false

Anything else you want to add:

* Non-bare clones still work when core.bare=false
* Bare clones only work when core.bare=true, or if core.bare is absent from gitconfig
* Started failing with Git 2.30.0, also fails on 2.30.1, but succeeds on 2.29.2
* Issue reproduced on multiple machines, running Ubuntu, Mac OS, and Alpine Linux
* Running with all TRACEs enabled shows only one additional line prior to "Cloning"
    15:38:08.567292 git.c:444  trace: built-in: git clone --bare https://github.com/josephvusich/fixture
* The fixture.git directory is created prior to the segfault, the fixture.git/config file content is below:
  [core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true

Please review the rest of the bug report below. You can delete any lines you don't wish to share.

[System Info] git version: git version 2.30.1 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Darwin 19.6.0 Darwin Kernel Version 19.6.0: Tue Jan 12 22:13:05 PST 2021; root:xnu-6153.141.16~1/RELEASE_X86_64 x86_64 compiler info: clang: 12.0.0 (clang-1200.0.32.2) libc info: no libc information available $SHELL (typically, interactive shell): /bin/zsh

[Enabled Hooks] not run from a git repository - no hooks to show

On 2021-03-03 at 00:06:01, Vusich, Joseph wrote:

I appreciate the report, and I can reproduce with 45526154a5 from next. We should definitely not segfault in this case. I completely agree that segfaulting is the wrong behavior (because it's always the wrong behavior for a command-line tool).

I do, however, think we should either ignore core.bare in the global configuration, if we don't already, or produce an error. The documentation says, "If true this repository is assumed to be bare" and much like we ignore certain other settings outside of .git/config, we should ignore this one.

I also agree that "git clone --bare" should clone a bare repository since we'd ignore the global setting.

I'm probably not going to get a chance to look at this before the weekend, so anyone is free to come up with a patch in the meantime, but if nobody has gotten to it then, I'll try to send out a patch.

I will admit being a bit interested in how this was discovered, since it seems like an odd configuration to have, so if you can share, I'd appreciate it, if only to satisfy my curiosity.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

I had the same reaction. Forcing everything to be bare is quite unusual.

Quite by accident. A machine-generated .gitconfig with some "default" values, which inadvertently included core.bare=false.

Technically non-bare in this case, although the field seems to be ignored in the global config, at least prior to Git 2.30.0. In any case, it never had any observable effect, and was therefore never noticed.

In 552955ed7f ("clone: use more conventional config/option layering", 2020-10-01), clone learned to read configuration options earlier in its execution, before creating the new repository. However, that led to a problem: if the core.bare setting is set to false in the global config, cloning a bare repository segfaults. This happens because the repository is falsely thought to be non-bare, but clone has set the work tree to NULL, which is then dereferenced.

The code to initialize the repository already considers the fact that a user might want to override the --bare option for git init, but it doesn't take into account clone, which uses a different option. Let's just check that the work tree is not NULL, since that's how clone indicates that the repository is bare. This is also the case for git init, so we won't be regressing that case.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
It appears from my reading of the init code that we did intentionally
already honor core.bare in some case, so I have not implemented ignoring
that here and instead went for the limited approach.

 builtin/init-db.c        | 4 ++--
 t/t5606-clone-options.sh | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

On Mon, Mar 8, 2021 at 8:18 AM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Perhaps this deserves a:

    Reported-by: Joseph Vusich <jvusich@amazon.com>

Can this be done more simply as:

    git clone --bare parent clone-bare &&

or even:

    git clone --bare . clone-bare &&

without mucking about with $(pwd)?

These days, we'd probably say:

    echo true >expect &&
    git -C clone-bare rev-parse --is-bare-repository >actual &&
    test_cmp expect actual

but it's subjective and minor; not at all worth a re-roll.

On 2021-03-08 at 16:43:58, Eric Sunshine wrote:

Good point.  Will fix.

Sure.  I pulled the line from the test above, but I agree that's nicer.

There's enough nits to warrant a v2, so I can do one.

In 552955ed7f ("clone: use more conventional config/option layering", 2020-10-01), clone learned to read configuration options earlier in its execution, before creating the new repository. However, that led to a problem: if the core.bare setting is set to false in the global config, cloning a bare repository segfaults. This happens because the repository is falsely thought to be non-bare, but clone has set the work tree to NULL, which is then dereferenced.

The code to initialize the repository already considers the fact that a user might want to override the --bare option for git init, but it doesn't take into account clone, which uses a different option. Let's just check that the work tree is not NULL, since that's how clone indicates that the repository is bare. This is also the case for git init, so we won't be regressing that case.

Reported-by: Joseph Vusich <jvusich@amazon.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/init-db.c        | 4 ++--
 t/t5606-clone-options.sh | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Because "git clone" does so many different things from the normal codepath in the "local" codepath, I'd prefer to see this one done with the "--no-local" option (or alternatively, we could test both, but that may be overkill).

On 2021-03-10 at 23:09:32, Junio C Hamano wrote:

I can reroll with that.