Skip to content

Commit d131482

Browse files
committed
Merge branch 'mh/loose-refs-race-with-pack-ref'
We read loose and packed rerferences in two steps, but after deciding to read a loose ref but before actually opening it to read it, another process racing with us can unlink it, which would cause us to barf. Update the codepath to retry when such a race is detected. * mh/loose-refs-race-with-pack-ref: resolve_ref_unsafe(): close race condition reading loose refs resolve_ref_unsafe(): handle the case of an SHA-1 within loop resolve_ref_unsafe(): extract function handle_missing_loose_ref()
2 parents 96ffd4c + fcb7c76 commit d131482

File tree

1 file changed

+72
-34
lines changed

1 file changed

+72
-34
lines changed

refs.c

Lines changed: 72 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,6 +1197,37 @@ static struct ref_entry *get_packed_ref(const char *refname)
11971197
return find_ref(get_packed_refs(&ref_cache), refname);
11981198
}
11991199

1200+
/*
1201+
* A loose ref file doesn't exist; check for a packed ref. The
1202+
* options are forwarded from resolve_safe_unsafe().
1203+
*/
1204+
static const char *handle_missing_loose_ref(const char *refname,
1205+
unsigned char *sha1,
1206+
int reading,
1207+
int *flag)
1208+
{
1209+
struct ref_entry *entry;
1210+
1211+
/*
1212+
* The loose reference file does not exist; check for a packed
1213+
* reference.
1214+
*/
1215+
entry = get_packed_ref(refname);
1216+
if (entry) {
1217+
hashcpy(sha1, entry->u.value.sha1);
1218+
if (flag)
1219+
*flag |= REF_ISPACKED;
1220+
return refname;
1221+
}
1222+
/* The reference is not a packed reference, either. */
1223+
if (reading) {
1224+
return NULL;
1225+
} else {
1226+
hashclr(sha1);
1227+
return refname;
1228+
}
1229+
}
1230+
12001231
const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag)
12011232
{
12021233
int depth = MAXDEPTH;
@@ -1221,36 +1252,34 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea
12211252

12221253
git_snpath(path, sizeof(path), "%s", refname);
12231254

1255+
/*
1256+
* We might have to loop back here to avoid a race
1257+
* condition: first we lstat() the file, then we try
1258+
* to read it as a link or as a file. But if somebody
1259+
* changes the type of the file (file <-> directory
1260+
* <-> symlink) between the lstat() and reading, then
1261+
* we don't want to report that as an error but rather
1262+
* try again starting with the lstat().
1263+
*/
1264+
stat_ref:
12241265
if (lstat(path, &st) < 0) {
1225-
struct ref_entry *entry;
1226-
1227-
if (errno != ENOENT)
1266+
if (errno == ENOENT)
1267+
return handle_missing_loose_ref(refname, sha1,
1268+
reading, flag);
1269+
else
12281270
return NULL;
1229-
/*
1230-
* The loose reference file does not exist;
1231-
* check for a packed reference.
1232-
*/
1233-
entry = get_packed_ref(refname);
1234-
if (entry) {
1235-
hashcpy(sha1, entry->u.value.sha1);
1236-
if (flag)
1237-
*flag |= REF_ISPACKED;
1238-
return refname;
1239-
}
1240-
/* The reference is not a packed reference, either. */
1241-
if (reading) {
1242-
return NULL;
1243-
} else {
1244-
hashclr(sha1);
1245-
return refname;
1246-
}
12471271
}
12481272

12491273
/* Follow "normalized" - ie "refs/.." symlinks by hand */
12501274
if (S_ISLNK(st.st_mode)) {
12511275
len = readlink(path, buffer, sizeof(buffer)-1);
1252-
if (len < 0)
1253-
return NULL;
1276+
if (len < 0) {
1277+
if (errno == ENOENT || errno == EINVAL)
1278+
/* inconsistent with lstat; retry */
1279+
goto stat_ref;
1280+
else
1281+
return NULL;
1282+
}
12541283
buffer[len] = 0;
12551284
if (!prefixcmp(buffer, "refs/") &&
12561285
!check_refname_format(buffer, 0)) {
@@ -1273,8 +1302,13 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea
12731302
* a ref
12741303
*/
12751304
fd = open(path, O_RDONLY);
1276-
if (fd < 0)
1277-
return NULL;
1305+
if (fd < 0) {
1306+
if (errno == ENOENT)
1307+
/* inconsistent with lstat; retry */
1308+
goto stat_ref;
1309+
else
1310+
return NULL;
1311+
}
12781312
len = read_in_full(fd, buffer, sizeof(buffer)-1);
12791313
close(fd);
12801314
if (len < 0)
@@ -1286,8 +1320,19 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea
12861320
/*
12871321
* Is it a symbolic ref?
12881322
*/
1289-
if (prefixcmp(buffer, "ref:"))
1290-
break;
1323+
if (prefixcmp(buffer, "ref:")) {
1324+
/*
1325+
* Please note that FETCH_HEAD has a second
1326+
* line containing other data.
1327+
*/
1328+
if (get_sha1_hex(buffer, sha1) ||
1329+
(buffer[40] != '\0' && !isspace(buffer[40]))) {
1330+
if (flag)
1331+
*flag |= REF_ISBROKEN;
1332+
return NULL;
1333+
}
1334+
return refname;
1335+
}
12911336
if (flag)
12921337
*flag |= REF_ISSYMREF;
12931338
buf = buffer + 4;
@@ -1300,13 +1345,6 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea
13001345
}
13011346
refname = strcpy(refname_buffer, buf);
13021347
}
1303-
/* Please note that FETCH_HEAD has a second line containing other data. */
1304-
if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {
1305-
if (flag)
1306-
*flag |= REF_ISBROKEN;
1307-
return NULL;
1308-
}
1309-
return refname;
13101348
}
13111349

13121350
char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)

0 commit comments

Comments
 (0)