@matheusbsbs / matheus-bagliano / commits / a8dd3821fe

csum-file: introduce hashfile_checkpoint_init()

In 106140a99f (builtin/fast-import: fix segfault with unsafe SHA1 backend, 2024-12-30) and 9218c0bfe1 (bulk-checkin: fix segfault with unsafe SHA1 backend, 2024-12-30), we observed the effects of failing to initialize a hashfile_checkpoint with the same hash function implementation as is used by the hashfile it is used to checkpoint. While both 106140a99f and 9218c0bfe1 work around the immediate crash, changing the hash function implementation within the hashfile API to, for example, the non-unsafe variant would re-introduce the crash. This is a result of the tight coupling between initializing hashfiles and hashfile_checkpoints. Introduce and use a new function which ensures that both parts of a hashfile and hashfile_checkpoint pair use the same hash function implementation to avoid such crashes. A few things worth noting: - In the change to builtin/fast-import.c::stream_blob(), we can see that by removing the explicit reference to 'the_hash_algo->unsafe_init_fn()', we are hardened against the hashfile API changing away from the_hash_algo (or its unsafe variant) in the future. - The bulk-checkin code no longer needs to explicitly zero-initialize the hashfile_checkpoint, since it is now done as a result of calling 'hashfile_checkpoint_init()'. - Also in the bulk-checkin code, we add an additional call to prepare_to_stream() outside of the main loop in order to initialize 'state->f' so we know which hash function implementation to use when calling 'hashfile_checkpoint_init()'. This is OK, since subsequent 'prepare_to_stream()' calls are noops. However, we only need to call 'prepare_to_stream()' when we have the HASH_WRITE_OBJECT bit set in our flags. Without that bit, calling 'prepare_to_stream()' does not assign 'state->f', so we have nothing to initialize. - Other uses of the 'checkpoint' in 'deflate_blob_to_pack()' are appropriately guarded. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Jan 23, 2025 at 12:34 UTC a8dd3821fe4fcf1524537ef97e4f5e2cf68ce949

4 files changed +15 -4

builtin/fast-import.c

+1 -1

@@ -1106,7 +1106,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)

1106 || (pack_size + PACK_SIZE_THRESHOLD + len) < pack_size)

1107 cycle_packfile();

1108

1109 - the_hash_algo->unsafe_init_fn(&checkpoint.ctx);

1109 + hashfile_checkpoint_init(pack_file, &checkpoint);

1110 hashfile_checkpoint(pack_file, &checkpoint);

1111 offset = checkpoint.offset;

1112

bulk-checkin.c

+6 -3

@@ -261,7 +261,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,

261 git_hash_ctx ctx;

262 unsigned char obuf[16384];

263 unsigned header_len;

264 - struct hashfile_checkpoint checkpoint = {0};

264 + struct hashfile_checkpoint checkpoint;

265 struct pack_idx_entry *idx = NULL;

266

267 seekback = lseek(fd, 0, SEEK_CUR);

@@ -272,12 +272,15 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,

272 OBJ_BLOB, size);

273 the_hash_algo->init_fn(&ctx);

274 the_hash_algo->update_fn(&ctx, obuf, header_len);

275 - the_hash_algo->unsafe_init_fn(&checkpoint.ctx);

275

276 /* Note: idx is non-NULL when we are writing */

278 - if ((flags & HASH_WRITE_OBJECT) != 0)

277 + if ((flags & HASH_WRITE_OBJECT) != 0) {

278 CALLOC_ARRAY(idx, 1);

279

280 + prepare_to_stream(state, flags);

281 + hashfile_checkpoint_init(state->f, &checkpoint);

282 + }

283 +

284 already_hashed_to = 0;

285

286 while (1) {

csum-file.c

@@ -206,6 +206,13 @@ struct hashfile *hashfd_throughput(int fd, const char *name, struct progress *tp

206 return hashfd_internal(fd, name, tp, 8 * 1024);

207 }

208

209 +void hashfile_checkpoint_init(struct hashfile *f,

210 + struct hashfile_checkpoint *checkpoint)

211 +{

212 + memset(checkpoint, 0, sizeof(*checkpoint));

213 + f->algop->init_fn(&checkpoint->ctx);

214 +}

215 +

216 void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpoint)

217 {

218 hashflush(f);

csum-file.h

@@ -36,6 +36,7 @@ struct hashfile_checkpoint {

36 git_hash_ctx ctx;

37 };

39 +void hashfile_checkpoint_init(struct hashfile *, struct hashfile_checkpoint *);

40 void hashfile_checkpoint(struct hashfile *, struct hashfile_checkpoint *);

41 int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);