[PATCH] Make locked paths absolute when current directory is changed
From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 23:01:58
Locked paths are saved in a linked list so that if something wrong happens, *.lock are removed. This works fine if we keep cwd the same, which is true 99% of time except: - update-index and read-tree hold the lock on $GIT_DIR/index really early, then later on may call setup_work_tree() to move cwd. - Suppose a lock is being held (e.g. by "git add") then somewhere down the line, somebody calls real_path (e.g. "link_alt_odb_entry"), which temporarily moves cwd away and back. During that time when cwd is moved (either permanently or temporarily) and we decide to die(), attempts to remove relative *.lock will fail, and the next operation will complain that some files are still locked. Avoid this case by turning relative paths to absolute when chdir() is called (or soon to be called, in setup_git_directory_gently case). Reported-by: Yue Lin Ho <redacted> Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- It occurred to me while writing this commit message that it would be better if we can unlink via a file descriptor so we don't have to convert paths. But I'm not sure about the availability of unlinkat(), especially on Windows. abspath.c | 2 +- cache.h | 6 ++++++ git.c | 6 +++--- lockfile.c | 16 ++++++++++++++++ path.c | 4 ++-- run-command.c | 2 +- setup.c | 3 ++- unix-socket.c | 2 +- 8 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/abspath.c b/abspath.c
index ca33558..78c963f 100644
--- a/abspath.c
+++ b/abspath.c@@ -87,7 +87,7 @@ static const char *real_path_internal(const char *path, int die_on_error) goto error_out; } - if (chdir(buf)) { + if (chdir_safe(buf)) { if (die_on_error) die_errno("Could not switch to '%s'", buf); else
diff --git a/cache.h b/cache.h
index 44aa439..d3f2596 100644
--- a/cache.h
+++ b/cache.h@@ -564,6 +564,12 @@ extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); extern int commit_lock_file(struct lock_file *); extern void update_index_if_able(struct index_state *, struct lock_file *); +extern void make_locked_paths_absolute(void); +static inline int chdir_safe(const char *path) +{ + make_locked_paths_absolute(); + return chdir(path); +} extern int hold_locked_index(struct lock_file *, int); extern int commit_locked_index(struct lock_file *);
diff --git a/git.c b/git.c
index 5b6c761..27766c3 100644
--- a/git.c
+++ b/git.c@@ -48,7 +48,7 @@ static void save_env(void) static void restore_env(void) { int i; - if (*orig_cwd && chdir(orig_cwd)) + if (*orig_cwd && chdir_safe(orig_cwd)) die_errno("could not move to %s", orig_cwd); for (i = 0; i < ARRAY_SIZE(env_names); i++) { if (orig_env[i])
@@ -206,7 +206,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) fprintf(stderr, "No directory given for -C.\n" ); usage(git_usage_string); } - if (chdir((*argv)[1])) + if (chdir_safe((*argv)[1])) die_errno("Cannot change to '%s'", (*argv)[1]); if (envchanged) *envchanged = 1;
@@ -292,7 +292,7 @@ static int handle_alias(int *argcp, const char ***argv) ret = 1; } - if (subdir && chdir(subdir)) + if (subdir && chdir_safe(subdir)) die_errno("Cannot change to '%s'", subdir); errno = saved_errno;
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..a70d107 100644
--- a/lockfile.c
+++ b/lockfile.c@@ -280,3 +280,19 @@ void rollback_lock_file(struct lock_file *lk) } lk->filename[0] = 0; } + +void make_locked_paths_absolute(void) +{ + struct lock_file *lk; + const char *abspath; + for (lk = lock_file_list; lk != NULL; lk = lk->next) { + if (!lk->filename[0] || lk->filename[0] == '/') + continue; + abspath = absolute_path(lk->filename); + if (strlen(abspath) >= sizeof(lk->filename)) + warning("locked path %s is relative when current directory " + "is changed", lk->filename); + else + strcpy(lk->filename, abspath); + } +}
diff --git a/path.c b/path.c
index bc804a3..9e8e101 100644
--- a/path.c
+++ b/path.c@@ -372,11 +372,11 @@ const char *enter_repo(const char *path, int strict) gitfile = read_gitfile(used_path) ; if (gitfile) strcpy(used_path, gitfile); - if (chdir(used_path)) + if (chdir_safe(used_path)) return NULL; path = validated_path; } - else if (chdir(path)) + else if (chdir_safe(path)) return NULL; if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
diff --git a/run-command.c b/run-command.c
index be07d4a..55f360e 100644
--- a/run-command.c
+++ b/run-command.c@@ -399,7 +399,7 @@ fail_pipe: close(cmd->out); } - if (cmd->dir && chdir(cmd->dir)) + if (cmd->dir && chdir_safe(cmd->dir)) die_errno("exec '%s': cd to '%s' failed", cmd->argv[0], cmd->dir); if (cmd->env) {
diff --git a/setup.c b/setup.c
index 0a22f8b..921045e 100644
--- a/setup.c
+++ b/setup.c@@ -290,7 +290,7 @@ void setup_work_tree(void) git_dir = get_git_dir(); if (!is_absolute_path(git_dir)) git_dir = real_path(get_git_dir()); - if (!work_tree || chdir(work_tree)) + if (!work_tree || chdir_safe(work_tree)) die("This operation must be run in a work tree"); /*
@@ -636,6 +636,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok) die_errno("Unable to read current working directory"); offset = len = strlen(cwd); + make_locked_paths_absolute(); /* * If GIT_DIR is set explicitly, we're not going * to do any discovery, but we still do repository
diff --git a/unix-socket.c b/unix-socket.c
index 01f119f..eeb8007 100644
--- a/unix-socket.c
+++ b/unix-socket.c@@ -12,7 +12,7 @@ static int unix_stream_socket(void) static int chdir_len(const char *orig, int len) { char *path = xmemdupz(orig, len); - int r = chdir(path); + int r = chdir_safe(path); free(path); return r; }
--
1.9.1.346.ga2b5940