git-commit-vandalism/pack-check.c

198 lines
5.3 KiB
C
Raw Normal View History

#include "cache.h"
#include "repository.h"
#include "pack.h"
#include "pack-revindex.h"
#include "progress.h"
#include "packfile.h"
#include "object-store.h"
struct idx_entry {
off_t offset;
unsigned int nr;
};
static int compare_entries(const void *e1, const void *e2)
{
const struct idx_entry *entry1 = e1;
const struct idx_entry *entry2 = e2;
if (entry1->offset < entry2->offset)
return -1;
if (entry1->offset > entry2->offset)
return 1;
return 0;
}
int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
off_t offset, off_t len, unsigned int nr)
{
const uint32_t *index_crc;
uint32_t data_crc = crc32(0, NULL, 0);
do {
2011-06-10 20:52:15 +02:00
unsigned long avail;
void *data = use_pack(p, w_curs, offset, &avail);
if (avail > len)
avail = len;
data_crc = crc32(data_crc, data, avail);
offset += avail;
len -= avail;
} while (len);
index_crc = p->index_data;
compute pack .idx byte offsets using size_t A pack and its matching .idx file are limited to 2^32 objects, because the pack format contains a 32-bit field to store the number of objects. Hence we use uint32_t in the code. But the byte count of even a .idx file can be much larger than that, because it stores at least a hash and an offset for each object. So using SHA-1, a v2 .idx file will cross the 4GB boundary at 153,391,650 objects. This confuses load_idx(), which computes the minimum size like this: unsigned long min_size = 8 + 4*256 + nr*(hashsz + 4 + 4) + hashsz + hashsz; Even though min_size will be big enough on most 64-bit platforms, the actual arithmetic is done as a uint32_t, resulting in a truncation. We actually exceed that min_size, but then we do: unsigned long max_size = min_size; if (nr) max_size += (nr - 1)*8; to account for the variable-sized table. That computation doesn't overflow quite so low, but with the truncation for min_size, we end up with a max_size that is much smaller than our actual size. So we complain that the idx is invalid, and can't find any of its objects. We can fix this case by casting "nr" to a size_t, which will do the multiplication in 64-bits (assuming you're on a 64-bit platform; this will never work on a 32-bit system since we couldn't map the whole .idx anyway). Likewise, we don't have to worry about further additions, because adding a smaller number to a size_t will convert the other side to a size_t. A few notes: - obviously we could just declare "nr" as a size_t in the first place (and likewise, packed_git.num_objects). But it's conceptually a uint32_t because of the on-disk format, and we correctly treat it that way in other contexts that don't need to compute byte offsets (e.g., iterating over the set of objects should and generally does use a uint32_t). Switching to size_t would make all of those other cases look wrong. - it could be argued that the proper type is off_t to represent the file offset. But in practice the .idx file must fit within memory, because we mmap the whole thing. And the rest of the code (including the idx_size variable we're comparing against) uses size_t. - we'll add the same cast to the max_size arithmetic line. Even though we're adding to a larger type, which will convert our result, the multiplication is still done as a 32-bit value and can itself overflow. I didn't check this with my test case, since it would need an even larger pack (~530M objects), but looking at compiler output shows that it works this way. The standard should agree, but I couldn't find anything explicit in 6.3.1.8 ("usual arithmetic conversions"). The case in load_idx() was the most immediate one that I was able to trigger. After fixing it, looking up actual objects (including the very last one in sha1 order) works in a test repo with 153,725,110 objects. That's because bsearch_hash() works with uint32_t entry indices, and the actual byte access: int cmp = hashcmp(table + mi * stride, sha1); is done with "stride" as a size_t, causing the uint32_t "mi" to be promoted to a size_t. This is the way most code will access the index data. However, I audited all of the other byte-wise accesses of packed_git.index_data, and many of the others are suspect (they are similar to the max_size one, where we are adding to a properly sized offset or directly to a pointer, but the multiplication in the sub-expression can overflow). I didn't trigger any of these in practice, but I believe they're potential problems, and certainly adding in the cast is not going to hurt anything here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-13 06:06:48 +01:00
index_crc += 2 + 256 + (size_t)p->num_objects * (the_hash_algo->rawsz/4) + nr;
return data_crc != ntohl(*index_crc);
}
static int verify_packfile(struct repository *r,
struct packed_git *p,
struct pack_window **w_curs,
verify_fn fn,
struct progress *progress, uint32_t base_count)
{
off_t index_size = p->index_size;
const unsigned char *index_base = p->index_data;
git_hash_ctx ctx;
unsigned char hash[GIT_MAX_RAWSZ], *pack_sig;
off_t offset = 0, pack_sig_ofs = 0;
uint32_t nr_objects, i;
int err = 0;
struct idx_entry *entries;
if (!is_pack_valid(p))
return error("packfile %s cannot be accessed", p->pack_name);
r->hash_algo->init_fn(&ctx);
do {
2011-06-10 20:52:15 +02:00
unsigned long remaining;
unsigned char *in = use_pack(p, w_curs, offset, &remaining);
offset += remaining;
if (!pack_sig_ofs)
pack_sig_ofs = p->pack_size - r->hash_algo->rawsz;
if (offset > pack_sig_ofs)
remaining -= (unsigned int)(offset - pack_sig_ofs);
r->hash_algo->update_fn(&ctx, in, remaining);
} while (offset < pack_sig_ofs);
r->hash_algo->final_fn(hash, &ctx);
pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);
if (!hasheq(hash, pack_sig))
err = error("%s pack checksum mismatch",
p->pack_name);
if (!hasheq(index_base + index_size - r->hash_algo->hexsz, pack_sig))
err = error("%s pack checksum does not match its index",
p->pack_name);
unuse_pack(w_curs);
/* Make sure everything reachable from idx is valid. Since we
* have verified that nr_objects matches between idx and pack,
* we do not do scan-streaming check on the pack file.
*/
nr_objects = p->num_objects;
ALLOC_ARRAY(entries, nr_objects + 1);
entries[nr_objects].offset = pack_sig_ofs;
/* first sort entries by pack offset, since unpacking them is more efficient that way */
for (i = 0; i < nr_objects; i++) {
entries[i].offset = nth_packed_object_offset(p, i);
entries[i].nr = i;
}
QSORT(entries, nr_objects, compare_entries);
for (i = 0; i < nr_objects; i++) {
void *data;
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
struct object_id oid;
enum object_type type;
unsigned long size;
off_t curpos;
int data_valid;
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
if (nth_packed_object_id(&oid, p, entries[i].nr) < 0)
BUG("unable to get oid of object %lu from %s",
(unsigned long)entries[i].nr, p->pack_name);
if (p->index_version > 1) {
off_t offset = entries[i].offset;
off_t len = entries[i+1].offset - offset;
unsigned int nr = entries[i].nr;
if (check_pack_crc(p, w_curs, offset, len, nr))
err = error("index CRC mismatch for object %s "
"from %s at offset %"PRIuMAX"",
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
oid_to_hex(&oid),
p->pack_name, (uintmax_t)offset);
}
curpos = entries[i].offset;
type = unpack_object_header(p, w_curs, &curpos, &size);
unuse_pack(w_curs);
if (type == OBJ_BLOB && big_file_threshold <= size) {
/*
* Let stream_object_signature() check it with
* the streaming interface; no point slurping
* the data in-core only to discard.
*/
data = NULL;
data_valid = 0;
} else {
data = unpack_entry(r, p, entries[i].offset, &type, &size);
data_valid = 1;
}
if (data_valid && !data)
err = error("cannot unpack %s from %s at offset %"PRIuMAX"",
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
oid_to_hex(&oid), p->pack_name,
(uintmax_t)entries[i].offset);
else if (data && check_object_signature(r, &oid, data, size,
type) < 0)
err = error("packed %s from %s is corrupt",
oid_to_hex(&oid), p->pack_name);
else if (!data && stream_object_signature(r, &oid) < 0)
err = error("packed %s from %s is corrupt",
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
oid_to_hex(&oid), p->pack_name);
else if (fn) {
int eaten = 0;
pack-check: push oid lookup into loop When we're checking a pack with fsck or verify-pack, we first sort the idx entries by offset, since accessing them in pack order is more efficient. To do so, we loop over them and fill in an array of structs with the offset, object_id, and index position of each, sort the result, and only then do we iterate over the sorted array and process each entry. In order to avoid the memory cost of storing the hash of each object, we just store a pointer into the copy in the mmap'd pack index file. To keep that property even as the rest of the code converted to "struct object_id", commit 9fd750461b (Convert the verify_pack callback to struct object_id, 2017-05-06) introduced a union in order to type-pun the pointer-to-hash into an object_id struct. But we can make this even simpler by observing that the sort operation doesn't need the object id at all! We only need them one at a time while we actually process each entry. So we can just omit the oid from the struct entirely and load it on the fly into a local variable in the second loop. This gets rid of the type-punning, and lets us directly use the more type-safe nth_packed_object_id(), simplifying the code. And as a bonus, it saves 8 bytes of memory per object. Note that this does mean we'll do the offset lookup for each object before the oid lookup. The oid lookup has more safety checks in it (e.g., for looking past p->num_objects) which in theory protected the offset lookup. But since violating those checks was already a BUG() condition (as described in the previous commit), it's not worth worrying about. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 05:36:31 +01:00
err |= fn(&oid, type, size, data, &eaten);
if (eaten)
data = NULL;
}
if (((base_count + i) & 1023) == 0)
display_progress(progress, base_count + i);
free(data);
}
display_progress(progress, base_count + i);
free(entries);
return err;
}
int verify_pack_index(struct packed_git *p)
{
int err = 0;
if (open_pack_index(p))
return error("packfile %s index not opened", p->pack_name);
/* Verify SHA1 sum of the index file */
if (!hashfile_checksum_valid(p->index_data, p->index_size))
err = error("Packfile index for %s hash mismatch",
p->pack_name);
return err;
}
int verify_pack(struct repository *r, struct packed_git *p, verify_fn fn,
struct progress *progress, uint32_t base_count)
{
int err = 0;
struct pack_window *w_curs = NULL;
err |= verify_pack_index(p);
if (!p->index_data)
return -1;
err |= verify_packfile(r, p, &w_curs, fn, progress, base_count);
unuse_pack(&w_curs);
return err;
}