Download raw body.
check for size_t overflow when mapping memory
On 2022/10/24 22:13:48 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Oct 24, 2022 at 09:57:38PM +0200, Omar Polo wrote:
> > #ifndef GOT_PACK_NO_MMAP
> > - if (hdrlen + size > 0) {
> > - (*obj)->data = mmap(NULL, hdrlen + size, PROT_READ,
> > + (*obj)->fd = *outfd;
> > + *outfd = -1;
> > + if (tot > 0 && tot < SIZE_MAX) {
> > + (*obj)->data = mmap(NULL, tot, PROT_READ,
> > MAP_PRIVATE, *outfd, 0);
> > if ((*obj)->data == MAP_FAILED) {
> > if (errno != ENOMEM) {
> > @@ -958,9 +963,6 @@ got_object_raw_alloc(struct got_raw_object **obj, uint
> > goto done;
> > }
> > (*obj)->data = NULL;
> > - } else {
> > - (*obj)->fd = *outfd;
> > - *outfd = -1;
> > }
> > }
> > #endif
>
> Won't setting *outfd to -1 in all cases disable the fallback to
> stdio via obj->f (which is a FILE *) in case mmap fails?
> The lines below this are as follow, and they would never run with
> your change in place:
>
> if (*outfd != -1) {
> (*obj)->f = fdopen(*outfd, "r");
> if ((*obj)->f == NULL) {
> err = got_error_from_errno("fdopen");
> goto done;
> }
> *outfd = -1;
> }
yup, that change slipped from a previous attempt, but it's clearly
wrong. i should have rechecked that part, apologise.
> > blob - becc8ffd68217466f0bad6d811196c0d7137e4df
> > blob + 0b79146e747f73787b2ee568b7fb74c517ce1d9e
> > --- lib/pack.c
> > +++ lib/pack.c
> > @@ -392,13 +392,15 @@ got_packidx_open(struct got_packidx **packidx,
> > }
> >
> > #ifndef GOT_PACK_NO_MMAP
> > - p->map = mmap(NULL, p->len, PROT_READ, MAP_PRIVATE, p->fd, 0);
> > - if (p->map == MAP_FAILED) {
> > - if (errno != ENOMEM) {
> > - err = got_error_from_errno("mmap");
> > - goto done;
> > + if (p->len < SIZE_MAX) {
>
> Not a big deal, but I wonder if this should not say <= SIZE_MAX?
> All such checks should also ensure that file size is greater 0.
woops, right! fixed, thanks!
diff f7b5105d849a571a34d8e5c2674b776679e9b5dd 24933640815f1defd057122da353993e1a60f843
commit - f7b5105d849a571a34d8e5c2674b776679e9b5dd
commit + 24933640815f1defd057122da353993e1a60f843
blob - f8e685448165384cfc578c0f089b4aa6fb6219c8
blob + 7bc7eb3dcdcec57157f35f8b6c676e1979ba2b76
--- lib/object.c
+++ lib/object.c
@@ -926,7 +926,10 @@ got_object_raw_alloc(struct got_raw_object **obj, uint
size_t hdrlen, off_t size)
{
const struct got_error *err = NULL;
+ off_t tot;
+ tot = hdrlen + size;
+
*obj = calloc(1, sizeof(**obj));
if (*obj == NULL) {
err = got_error_from_errno("calloc");
@@ -944,13 +947,13 @@ got_object_raw_alloc(struct got_raw_object **obj, uint
goto done;
}
- if (sb.st_size != hdrlen + size) {
+ if (sb.st_size != tot) {
err = got_error(GOT_ERR_PRIVSEP_LEN);
goto done;
}
#ifndef GOT_PACK_NO_MMAP
- if (hdrlen + size > 0) {
- (*obj)->data = mmap(NULL, hdrlen + size, PROT_READ,
+ if (tot > 0 && tot <= SIZE_MAX) {
+ (*obj)->data = mmap(NULL, tot, PROT_READ,
MAP_PRIVATE, *outfd, 0);
if ((*obj)->data == MAP_FAILED) {
if (errno != ENOMEM) {
blob - becc8ffd68217466f0bad6d811196c0d7137e4df
blob + 5e8b181042e11b8efbc129bd3a9cfb8c448138d3
--- lib/pack.c
+++ lib/pack.c
@@ -392,13 +392,15 @@ got_packidx_open(struct got_packidx **packidx,
}
#ifndef GOT_PACK_NO_MMAP
- p->map = mmap(NULL, p->len, PROT_READ, MAP_PRIVATE, p->fd, 0);
- if (p->map == MAP_FAILED) {
- if (errno != ENOMEM) {
- err = got_error_from_errno("mmap");
- goto done;
+ if (p->len > 0 && p->len <= SIZE_MAX) {
+ p->map = mmap(NULL, p->len, PROT_READ, MAP_PRIVATE, p->fd, 0);
+ if (p->map == MAP_FAILED) {
+ if (errno != ENOMEM) {
+ err = got_error_from_errno("mmap");
+ goto done;
+ }
+ p->map = NULL; /* fall back to read(2) */
}
- p->map = NULL; /* fall back to read(2) */
}
#endif
@@ -1036,7 +1038,6 @@ resolve_offset_delta(struct got_delta_chain *deltas,
resolve_offset_delta(struct got_delta_chain *deltas,
struct got_packidx *packidx, struct got_pack *pack, off_t delta_offset,
size_t tslen, int delta_type, size_t delta_size, unsigned int recursion)
-
{
const struct got_error *err;
off_t base_offset;
blob - b07bf0c9c711997031a7d3afa3388a27527ef2db
blob + 50237bbf7e4f90efc06e68c84cedd6c84631551e
--- lib/repository.c
+++ lib/repository.c
@@ -1428,14 +1428,16 @@ got_repo_cache_pack(struct got_pack **packp, struct go
goto done;
#ifndef GOT_PACK_NO_MMAP
- pack->map = mmap(NULL, pack->filesize, PROT_READ, MAP_PRIVATE,
- pack->fd, 0);
- if (pack->map == MAP_FAILED) {
- if (errno != ENOMEM) {
- err = got_error_from_errno("mmap");
- goto done;
+ if (pack->filesize > 0 && pack->filesize <= SIZE_MAX) {
+ pack->map = mmap(NULL, pack->filesize, PROT_READ, MAP_PRIVATE,
+ pack->fd, 0);
+ if (pack->map == MAP_FAILED) {
+ if (errno != ENOMEM) {
+ err = got_error_from_errno("mmap");
+ goto done;
+ }
+ pack->map = NULL; /* fall back to read(2) */
}
- pack->map = NULL; /* fall back to read(2) */
}
#endif
done:
blob - 8a38382357c6aeb5b642965a26a50d9a6621d34e
blob + 7fda0c4e47f3539c4b13427d388cc26e749b75bb
--- libexec/got-index-pack/got-index-pack.c
+++ libexec/got-index-pack/got-index-pack.c
@@ -183,10 +183,12 @@ main(int argc, char **argv)
}
#ifndef GOT_PACK_NO_MMAP
- pack.map = mmap(NULL, pack.filesize, PROT_READ, MAP_PRIVATE,
- pack.fd, 0);
- if (pack.map == MAP_FAILED)
- pack.map = NULL; /* fall back to read(2) */
+ if (pack.filesize > 0 && pack.filesize <= SIZE_MAX) {
+ pack.map = mmap(NULL, pack.filesize, PROT_READ, MAP_PRIVATE,
+ pack.fd, 0);
+ if (pack.map == MAP_FAILED)
+ pack.map = NULL; /* fall back to read(2) */
+ }
#endif
err = got_pack_index(&pack, idxfd, tmpfiles[0], tmpfiles[1],
tmpfiles[2], pack_hash, send_index_pack_progress, &ibuf, &rl);
blob - 268ab18c5371e622b7f9c3a62c9a894a7050de85
blob + e49fd95ba0b7919efe9005b306782db008b22370
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1146,9 +1146,11 @@ receive_packidx(struct got_packidx **packidx, struct i
}
#ifndef GOT_PACK_NO_MMAP
- p->map = mmap(NULL, p->len, PROT_READ, MAP_PRIVATE, p->fd, 0);
- if (p->map == MAP_FAILED)
- p->map = NULL; /* fall back to read(2) */
+ if (p->len > 0 && p->len <= SIZE_MAX) {
+ p->map = mmap(NULL, p->len, PROT_READ, MAP_PRIVATE, p->fd, 0);
+ if (p->map == MAP_FAILED)
+ p->map = NULL; /* fall back to read(2) */
+ }
#endif
err = got_packidx_init_hdr(p, 1, ipackidx.packfile_size);
done:
@@ -1874,10 +1876,12 @@ receive_pack(struct got_pack **packp, struct imsgbuf *
goto done;
#ifndef GOT_PACK_NO_MMAP
- pack->map = mmap(NULL, pack->filesize, PROT_READ, MAP_PRIVATE,
- pack->fd, 0);
- if (pack->map == MAP_FAILED)
- pack->map = NULL; /* fall back to read(2) */
+ if (pack->filesize > 0 && pack->filesize <= SIZE_MAX) {
+ pack->map = mmap(NULL, pack->filesize, PROT_READ, MAP_PRIVATE,
+ pack->fd, 0);
+ if (pack->map == MAP_FAILED)
+ pack->map = NULL; /* fall back to read(2) */
+ }
#endif
done:
if (err) {
check for size_t overflow when mapping memory