mbox series

[0/1] maintenance: Fix a SEGFAULT when no repository when running git maintenance run/start

Message ID 20201124164405.29327-1-rafaeloliveira.cs@gmail.com (mailing list archive)
Headers show
Series maintenance: Fix a SEGFAULT when no repository when running git maintenance run/start | expand

Message

Rafael Silva Nov. 24, 2020, 4:44 p.m. UTC 
In d7514f6ed5 (maintenance: take a lock on the objects directory, 2020-09-17) [1],
and 2fec604f8d (maintenance: add start/stop subcommands, 2020-09-11) [2] The
"git maintenance run" and "git maintenance start" was taught to hold a file-based
lock at the .git/maintenance.lock and .git/schedule.lock respectively because these
operations involves writing data into the .git/repository. 

The lock file path string is built using the the_repository->objects->odb->path,
in case the_repository->objects->odb is NULL when there is not repository available,
resulting in a SEGFAULT.

[1] https://lore.kernel.org/git/1a0a3eebb825ac3eabfdd86f82ed7ef6abb454c5.1600366313.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/5194f6b1facbd14cc17eea0337c0cc397a2a51fc.1602782524.git.gitgitgadget@gmail.com/

In order to reproduce the error, one can execute maintenance "run" and/or
"start" subcommand with a non valid repository: 

    $ git -C /tmp maintenance start
    Segmentation fault

    $ git -C /tmp maintenance run
    Segmentation fault

The above test was executed from a git built from commit: faefdd61ec (Sixth batch, 2020-11-18):

For reference here's the output from GDB when debugging the "start" command

	Program received signal SIGSEGV, Segmentation fault.
	0x00005555555b9b4c in maintenance_run_tasks (opts=0x7fffffffded4) at builtin/gc.c:1268
	1268		char *lock_path = xstrfmt("%s/maintenance", r->objects->odb->path);

This patch serie adds a check on the maintenance_run_tasks() and update_background_schedule()
to check if the_repository->git_dir is set to validate if a git repository is available and
return error otherwise. The logic is borrowed from maintenance_unregister() that performs
the same validation with different error message.

A new test case is added into t/t7900-maintenance.sh to cover these cases. The test is based on
the assumption that the `/tmp` directory is available, readable and is not a git repository
which I hope is fine for the running the test is all platforms. 

This patch is based on faefdd61ec (Sixth batch, 2020-11-18) (master branch) as the both commits
that introduced the file-based lock are graduated to master already. Hope this also plays nice
with the integration branches maintenance-part-v[1,4]. 

Rafael Silva (1):
  maintenance: fix a SEGFAULT when no repository

 builtin/gc.c           | 14 ++++++++++++--
 t/t7900-maintenance.sh |  5 +++++
 2 files changed, 17 insertions(+), 2 deletions(-)