"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
fix arith in pack_index / unbreak on 32 bit arches
To:
gameoftrees@openbsd.org
Date:
Sat, 10 Aug 2024 15:40:03 +0200

Download raw body.

Thread
Finally managed to understand why the gotd regress was was consistently
failing on my buildbot.  The issue is here:

lib/pack_index.c:
>806                 if (lseek(pack->fd, -digest_len, SEEK_END) == -1) {
 807                         err = got_error_from_errno("lseek");
 808                         goto done;
 809                 }
 810                 n = read(pack->fd, pack_hash_expected, digest_len);

My interpretation is that `-digest_len' (which is a size_t so 32bit on
x86) wraps and then gets extended to a off_t (64 bit i believe), so we
actually seek very, very far past the end of the file and then read(2)
returns zero.

Originally we did -SHA1_DIGEST_LENGTH, which is fine, but a careless
substitution with `digest_len' plus testing only on 64 bit arches has
hidden this problem.

With this my buildbot seems happy, gotd regress passes fully, cmdline is
still running but seems fine (takes a looong time :-) and tog mostly
passes (there's a failure in one test but seems independent from this,
i'll look into it and post another thread.)

ok?

diff /home/op/w/got
commit - faf51db5e8152629d9c4aa4672b3f26e6acecf10
path + /home/op/w/got
blob - f5c3ffa28603a655efaca764f0ff6031de2ef9f5
file + lib/pack_index.c
--- lib/pack_index.c
+++ lib/pack_index.c
@@ -627,7 +627,7 @@ got_pack_index(struct got_pack *pack, int idxfd, FILE 
 	size_t mapoff = 0;
 	int p_indexed = 0, last_p_indexed = -1;
 	int p_resolved = 0, last_p_resolved = -1;
-	size_t digest_len = got_hash_digest_length(pack->algo);
+	ssize_t digest_len = got_hash_digest_length(pack->algo);
 
 	/* Require that pack file header and hash trailer are present. */
 	if (pack->filesize < sizeof(hdr) + digest_len)