From: Mark Jamsek Subject: Re: fix arith in pack_index / unbreak on 32 bit arches To: Omar Polo Cc: gameoftrees@openbsd.org Date: Sun, 11 Aug 2024 00:32:29 +1000 Omar Polo wrote: > 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? Your diagnosis sounds right to me. A tiny style nit (annotated inline) that was already there which you might want to touch up while here but ok either way. > 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); changed line could remove function call in initialiser: ssize_t digest_len; 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) -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68