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>

Yiannis Marangos committed Apr 10, 2014 at 21:31 UTC 426ddeead6112955dfb50ccf9bb4af05d1ca9082
3 files changed +69 -1
cache.h
+3
@@ -279,6 +279,7 @@ struct index_state {
279 initialized : 1;
280 struct hash_table name_hash;
281 struct hash_table dir_hash;
282 + unsigned char sha1[20];
283 };
284
285 extern struct index_state the_index;
@@ -1199,6 +1200,8 @@ extern void fsync_or_die(int fd, const char *);
1200
1201 extern ssize_t read_in_full(int fd, void *buf, size_t count);
1202 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
1203 +extern ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
1204 +
1205 static inline ssize_t write_str_in_full(int fd, const char *str)
1206 {
1207 return write_in_full(fd, str, strlen(str));
read-cache.c
+46 -1
@@ -1464,6 +1464,7 @@ int read_index_from(struct index_state *istate, const char *path)
1464 if (verify_hdr(hdr, mmap_size) < 0)
1465 goto unmap;
1466
1467 + hashcpy(istate->sha1, (unsigned char *)hdr + mmap_size - 20);
1468 istate->version = ntohl(hdr->hdr_version);
1469 istate->cache_nr = ntohl(hdr->hdr_entries);
1470 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,
1748 return result;
1749 }
1750
1751 +/*
1752 + * This function verifies if index_state has the correct sha1 of the
1753 + * index file. Don't die if we have any other failure, just return 0.
1754 + */
1755 +static int verify_index_from(const struct index_state *istate, const char *path)
1756 +{
1757 + int fd;
1758 + ssize_t n;
1759 + struct stat st;
1760 + unsigned char sha1[20];
1761 +
1762 + if (!istate->initialized)
1763 + return 0;
1764 +
1765 + fd = open(path, O_RDONLY);
1766 + if (fd < 0)
1767 + return 0;
1768 +
1769 + if (fstat(fd, &st))
1770 + goto out;
1771 +
1772 + if (st.st_size < sizeof(struct cache_header) + 20)
1773 + goto out;
1774 +
1775 + n = pread_in_full(fd, sha1, 20, st.st_size - 20);
1776 + if (n != 20)
1777 + goto out;
1778 +
1779 + if (hashcmp(istate->sha1, sha1))
1780 + goto out;
1781 +
1782 + close(fd);
1783 + return 1;
1784 +
1785 +out:
1786 + close(fd);
1787 + return 0;
1788 +}
1789 +
1790 +static int verify_index(const struct index_state *istate)
1791 +{
1792 + return verify_index_from(istate, get_index_file());
1793 +}
1794 +
1795 static int has_racy_timestamp(struct index_state *istate)
1796 {
1797 int entries = istate->cache_nr;
@@ -1766,7 +1811,7 @@ static int has_racy_timestamp(struct index_state *istate)
1811 void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)
1812 {
1813 if ((istate->cache_changed || has_racy_timestamp(istate)) &&
1769 - !write_index(istate, lockfile->fd))
1814 + verify_index(istate) && !write_index(istate, lockfile->fd))
1815 commit_locked_index(lockfile);
1816 else
1817 rollback_lock_file(lockfile);
wrapper.c
+20
@@ -232,6 +232,26 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
232 return total;
233 }
234
235 +ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
236 +{
237 + char *p = buf;
238 + ssize_t total = 0;
239 +
240 + while (count > 0) {
241 + ssize_t loaded = xpread(fd, p, count, offset);
242 + if (loaded < 0)
243 + return -1;
244 + if (loaded == 0)
245 + return total;
246 + count -= loaded;
247 + p += loaded;
248 + total += loaded;
249 + offset += loaded;
250 + }
251 +
252 + return total;
253 +}
254 +
255 int xdup(int fd)
256 {
257 int ret = dup(fd);