read-cache.c: verify index file before we opportunistically update it

Before we proceed to opportunistically update the index (often done
by an otherwise read-only operation like "git status" and "git diff"
that internally refreshes the index), we must verify that the
current index file is the same as the one that we read earlier
before we took the lock on it, in order to avoid a possible race.

In the example below git-status does "opportunistic update" and
git-rebase updates the index, but the race can happen in general.

1. process A calls git-rebase (or does anything that uses the index)

2. process A applies 1st commit

3. process B calls git-status (or does anything that updates the index)

4. process B reads index

5. process A applies 2nd commit

6. process B takes the lock, then overwrites process A's changes.

7. process A applies 3rd commit

As an end result the 3rd commit will have a revert of the 2nd commit.
When process B takes the lock, it needs to make sure that the index
hasn't changed since step 4.

Signed-off-by: Yiannis Marangos <yiannis.marangos@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index ce377e1354a4d0fd719b50edb32124110956467f..9244c387c39b3a5cf29162a36fdb1388cff9a87f 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -279,6 +279,7 @@ struct index_state {
                 initialized : 1;
        struct hash_table name_hash;
        struct hash_table dir_hash;
                 initialized : 1;
        struct hash_table name_hash;
        struct hash_table dir_hash;

+       unsigned char sha1[20];

 };
 
 extern struct index_state the_index;
 };
 
 extern struct index_state the_index;


@@ -1199,6 +1200,8 @@ extern void fsync_or_die(int fd, const char *);
 
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);

+extern ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
+

 static inline ssize_t write_str_in_full(int fd, const char *str)
 {
        return write_in_full(fd, str, strlen(str));
 static inline ssize_t write_str_in_full(int fd, const char *str)
 {
        return write_in_full(fd, str, strlen(str));

diff --git a/read-cache.c b/read-cache.c
index 33dd676ccbbd24e0bace49347f1f5d81b5099bcc..f4a0d6168b8db2f91a9bbce035277dd2864a60d8 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -1464,6 +1464,7 @@ int read_index_from(struct index_state *istate, const char *path)
        if (verify_hdr(hdr, mmap_size) < 0)
                goto unmap;
 
        if (verify_hdr(hdr, mmap_size) < 0)
                goto unmap;
 

+       hashcpy(istate->sha1, (unsigned char *)hdr + mmap_size - 20);

        istate->version = ntohl(hdr->hdr_version);
        istate->cache_nr = ntohl(hdr->hdr_entries);
        istate->cache_alloc = alloc_nr(istate->cache_nr);
        istate->version = ntohl(hdr->hdr_version);
        istate->cache_nr = ntohl(hdr->hdr_entries);
        istate->cache_alloc = alloc_nr(istate->cache_nr);


@@ -1747,6 +1748,50 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
        return result;
 }
 
        return result;
 }
 

+/*
+ * This function verifies if index_state has the correct sha1 of the
+ * index file.  Don't die if we have any other failure, just return 0.
+ */
+static int verify_index_from(const struct index_state *istate, const char *path)
+{
+       int fd;
+       ssize_t n;
+       struct stat st;
+       unsigned char sha1[20];
+
+       if (!istate->initialized)
+               return 0;
+
+       fd = open(path, O_RDONLY);
+       if (fd < 0)
+               return 0;
+
+       if (fstat(fd, &st))
+               goto out;
+
+       if (st.st_size < sizeof(struct cache_header) + 20)
+               goto out;
+
+       n = pread_in_full(fd, sha1, 20, st.st_size - 20);
+       if (n != 20)
+               goto out;
+
+       if (hashcmp(istate->sha1, sha1))
+               goto out;
+
+       close(fd);
+       return 1;
+
+out:
+       close(fd);
+       return 0;
+}
+
+static int verify_index(const struct index_state *istate)
+{
+       return verify_index_from(istate, get_index_file());
+}
+

 static int has_racy_timestamp(struct index_state *istate)
 {
        int entries = istate->cache_nr;
 static int has_racy_timestamp(struct index_state *istate)
 {
        int entries = istate->cache_nr;


@@ -1766,7 +1811,7 @@ static int has_racy_timestamp(struct index_state *istate)
 void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)
 {
        if ((istate->cache_changed || has_racy_timestamp(istate)) &&
 void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)
 {
        if ((istate->cache_changed || has_racy_timestamp(istate)) &&

-           !write_index(istate, lockfile->fd))
+           verify_index(istate) && !write_index(istate, lockfile->fd))

                commit_locked_index(lockfile);
        else
                rollback_lock_file(lockfile);
                commit_locked_index(lockfile);
        else
                rollback_lock_file(lockfile);

diff --git a/wrapper.c b/wrapper.c
index 5b3c7fceb34c0aff6b1b798070b068bf71ed6b3f..bc1bfb86003cb4133cc4ce3ce6423ce780ed7c84 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -232,6 +232,26 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
        return total;
 }
 
        return total;
 }
 

+ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
+{
+       char *p = buf;
+       ssize_t total = 0;
+
+       while (count > 0) {
+               ssize_t loaded = xpread(fd, p, count, offset);
+               if (loaded < 0)
+                       return -1;
+               if (loaded == 0)
+                       return total;
+               count -= loaded;
+               p += loaded;
+               total += loaded;
+               offset += loaded;
+       }
+
+       return total;
+}
+

 int xdup(int fd)
 {
        int ret = dup(fd);
 int xdup(int fd)
 {
        int ret = dup(fd);