threads / patch / 36169

patchshallow: verify shallow file after taking lock

Subject: [PATCH] shallow: verify shallow file after taking lock

## tl;dr

One message between Mar 15, 2014 and Mar 15, 2014. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Jeff King· Mar 15, 2014, 03:47 UTC · lore

Before writing the shallow file, we stat() the existing file to make sure it has not been updated since our operation began. However, we do not do so under a lock, so there is a possible race:

  1. Process A takes the lock.
  2. Process B calls check_shallow_file_for_update and finds
     no update.
  3. Process A commits the lockfile.
  4. Process B takes the lock, then overwrite's process A's
     changes.
We can fix this by doing our check while we hold the lock.
Signed-off-by: Jeff King <peff@peff.net>
---
This is a repost of:
  http://article.gmane.org/gmane.comp.version-control.git/242795

You picked up the other two related patches as jk/shallow-update-fix, but I suspect this one just got lost in the noise because I didn't format it as a proper series.

 shallow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to shallow.c +2 −2
diff --git a/shallow.c b/shallow.c
index bbc98b5..75da07a 100644
--- a/shallow.c
+++ b/shallow.c
@@ -246,9 +246,9 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
-	check_shallow_file_for_update();
 	fd = hold_lock_file_for_update(shallow_lock, git_path("shallow"),
 				       LOCK_DIE_ON_ERROR);
+	check_shallow_file_for_update();
 	if (write_shallow_commits(&sb, 0, extra)) {
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
@@ -293,9 +293,9 @@ void prune_shallow(int show_only)
 		strbuf_release(&sb);
 		return;
 	}
-	check_shallow_file_for_update();
 	fd = hold_lock_file_for_update(&shallow_lock, git_path("shallow"),
 				       LOCK_DIE_ON_ERROR);
+	check_shallow_file_for_update();
 	if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
-- 
1.9.0.532.g9ab8f58

← back to recent threads