Skip to content
This repository was archived by the owner on Nov 9, 2017. It is now read-only.

Commit 426ddee

Browse files
obliquegitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 9aa91af commit 426ddee

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

cache.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ struct index_state {
279279
initialized : 1;
280280
struct hash_table name_hash;
281281
struct hash_table dir_hash;
282+
unsigned char sha1[20];
282283
};
283284

284285
extern struct index_state the_index;
@@ -1199,6 +1200,8 @@ extern void fsync_or_die(int fd, const char *);
11991200

12001201
extern ssize_t read_in_full(int fd, void *buf, size_t count);
12011202
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+
12021205
static inline ssize_t write_str_in_full(int fd, const char *str)
12031206
{
12041207
return write_in_full(fd, str, strlen(str));

read-cache.c

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,7 @@ int read_index_from(struct index_state *istate, const char *path)
14641464
if (verify_hdr(hdr, mmap_size) < 0)
14651465
goto unmap;
14661466

1467+
hashcpy(istate->sha1, (unsigned char *)hdr + mmap_size - 20);
14671468
istate->version = ntohl(hdr->hdr_version);
14681469
istate->cache_nr = ntohl(hdr->hdr_entries);
14691470
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,
17471748
return result;
17481749
}
17491750

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+
17501795
static int has_racy_timestamp(struct index_state *istate)
17511796
{
17521797
int entries = istate->cache_nr;
@@ -1766,7 +1811,7 @@ static int has_racy_timestamp(struct index_state *istate)
17661811
void update_index_if_able(struct index_state *istate, struct lock_file *lockfile)
17671812
{
17681813
if ((istate->cache_changed || has_racy_timestamp(istate)) &&
1769-
!write_index(istate, lockfile->fd))
1814+
verify_index(istate) && !write_index(istate, lockfile->fd))
17701815
commit_locked_index(lockfile);
17711816
else
17721817
rollback_lock_file(lockfile);

wrapper.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,26 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
232232
return total;
233233
}
234234

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+
235255
int xdup(int fd)
236256
{
237257
int ret = dup(fd);

0 commit comments

Comments
 (0)