From: "Todd C. Miller" Subject: Re: some size_t/off_t fixes for got-index-pack To: Stefan Sperling Cc: Omar Polo , gameoftrees@openbsd.org Date: Tue, 18 Oct 2022 08:58:34 -0600 On Tue, 18 Oct 2022 10:57:11 +0200, Stefan Sperling wrote: > So shouldn't mapoff remain a size_t? It describes an offset into the memory > map. And memory offsets should be size_t. > I would expect off_t to be reserved for cases where we address an offset > into a file accessed via a file descriptor. I think it should remain size_t since this code path uses mmap(2). > We cannot address the whole range of a mapped file if the file's size will no > t > fit into a size_t. So attempting to map such files should fail (I assume). > Note that mapoff is only used if pack->map is set. > For these reasons, my expectation is that assigning offset to mapoff is fine. > > Instead of changing the type of mapoff, should we add a range check before > copying the offset into mapoff, and return GOT_ERR_RANGE in case of overflow? > This would catch programming errors where we mix mapped i/o with regular i/o. Adding a range check seems like the best approach. I'm not sure whether GOT_ERR_PACK_OFFSET or GOT_ERR_RANGE is the more useful error. - todd