"GOT", but the "O" is a cute, smiling sun Index | Thread

From:
"Todd C. Miller" <millert@openbsd.org>
Subject:
Re: some size_t/off_t fixes for got-index-pack
To:
Stefan Sperling <stsp@stsp.name>
Cc:
Omar Polo <op@omarpolo.com>, gameoftrees@openbsd.org
Date:
Tue, 18 Oct 2022 08:58:34 -0600

Download raw body.

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