diff --git a/.gitignore b/.gitignore index 96cf545e8..ac9aef4a7 100644 --- a/.gitignore +++ b/.gitignore @@ -239,6 +239,7 @@ /tests/lib/pread.t /tests/lib/pwrite.t /tests/lib/qio.t +/tests/lib/readin.t /tests/lib/reallocarray.t /tests/lib/reservedfd.t /tests/lib/setenv.t @@ -261,9 +262,11 @@ /tests/overview/ovsqlite.t /tests/overview/ovsqlite-read.t /tests/overview/ovsqlite-write.t +/tests/overview/tdx-group.t /tests/overview/tradindexed.t /tests/overview/xref.t /tests/perl/minimum-version.t +/tests/storage/caf.t /tests/storage/cancel-tombstone.t /tests/util/innbind.t **/.libs/ diff --git a/MANIFEST b/MANIFEST index d641e64a6..c8e476eb6 100644 --- a/MANIFEST +++ b/MANIFEST @@ -1013,6 +1013,7 @@ tests/lib/network/server-t.c Tests for lib/network.c (server-oriented) tests/lib/pread-t.c Tests for lib/pread.c tests/lib/pwrite-t.c Tests for lib/pwrite.c tests/lib/qio-t.c Tests for lib/qio.c +tests/lib/readin-t.c Tests for lib/readin.c tests/lib/reallocarray-t.c Tests for lib/reallocarray.c tests/lib/reservedfd-t.c Tests for reserved file descriptors tests/lib/setenv-t.c Tests for lib/setenv.c @@ -1037,12 +1038,14 @@ tests/overview/ovsqlite-integ.t Integration test for ovsqlite direct reade tests/overview/ovsqlite-read-t.c Direct reader verification for integration test tests/overview/ovsqlite-t.c Unit tests for ovsqlite direct reader tests/overview/ovsqlite-write-t.c Writer helper for ovsqlite integration test +tests/overview/tdx-group-t.c Tests for tradindexed group index sizes tests/overview/xref-t.c Test storing overview data by Xref tests/perl Test suite for Perl scripts (Directory) tests/perl/minimum-version.t.in Tests for not too-new features of Perl tests/runtests.c The test suite driver program tests/storage Test suite for storage (Directory) tests/storage/archive.t Tests for backends/archive +tests/storage/caf-t.c Tests for CAF file validation and cleaning tests/storage/cancel-tombstone-t.c Tests for SMcanceltombstone tests/storage/makehistory.t Tests for expire/makehistory tests/storage/sm.t Tests for frontends/sm diff --git a/backends/innxmit.c b/backends/innxmit.c index 798b6cf2d..0c06cfd89 100644 --- a/backends/innxmit.c +++ b/backends/innxmit.c @@ -1062,14 +1062,21 @@ article_open(const char *path, const char *id) return NULL; if (fstat(fd, &st) < 0) { syswarn("requeue %s", path); + close(fd); + Requeue(path, id); + return NULL; + } + if (st.st_size < 0 || (uintmax_t) st.st_size > SIZE_MAX) { + warn("requeue %s: article is too large", path); + close(fd); Requeue(path, id); return NULL; } article = xmalloc(sizeof(ARTHANDLE)); article->type = TOKEN_EMPTY; - article->len = st.st_size; + article->len = (size_t) st.st_size; data = xmalloc(article->len); - if (xread(fd, data, article->len) < 0) { + if (xread(fd, data, st.st_size) < 0) { syswarn("requeue %s", path); free(data); free(article); @@ -1088,6 +1095,13 @@ article_open(const char *path, const char *id) } if (p[-1] != '\r') { p = wire_from_native(data, article->len, &length); + if (p == NULL) { + syswarn("requeue %s: cannot convert article", path); + free(data); + free(article); + Requeue(path, id); + return NULL; + } free(data); data = p; article->len = length; diff --git a/frontends/rnews.c b/frontends/rnews.c index 389d5b0c4..cba253e95 100644 --- a/frontends/rnews.c +++ b/frontends/rnews.c @@ -232,6 +232,11 @@ Process(char *article, size_t artlen) /* Convert the article to wire format. */ wirefmt = wire_from_native(article, artlen, &length); + if (wirefmt == NULL) { + Reject(article, artlen, "bad_article %s", + "too large after wire conversion"); + return true; + } /* Make sure that all the headers are there, note the ID. */ for (hp = RequiredHeaders; hp < ARRAY_END(RequiredHeaders); hp++) { diff --git a/frontends/sm.c b/frontends/sm.c index 8908ebc30..0b3f3d5d9 100644 --- a/frontends/sm.c +++ b/frontends/sm.c @@ -148,6 +148,8 @@ store_article(int fd) sysdie("cannot read article"); } text = wire_from_native(article->data, article->left, &size); + if (text == NULL) + sysdie("cannot convert article to wire format"); buffer_free(article); result = store_article_common(text, size); free(text); diff --git a/include/inn/wire.h b/include/inn/wire.h index 4bfe0aa38..be243eb92 100644 --- a/include/inn/wire.h +++ b/include/inn/wire.h @@ -46,7 +46,8 @@ char *wire_endheader(const char *header, const char *end); /* Given an article and length in non-wire format, return a malloced region containing the article in wire format and set newlen to the length of the - new article. */ + new article. Returns NULL with errno set to EOVERFLOW if the converted + article would be too large to represent. */ char *wire_from_native(const char *article, size_t len, size_t *newlen); /* Given an article and length in wire format, return a malloced region diff --git a/innd/icd.c b/innd/icd.c index 4174ef8be..8154fb582 100644 --- a/innd/icd.c +++ b/innd/icd.c @@ -454,7 +454,13 @@ ICDreadactive(char **endp) ICDactpath); exit(1); } - ICDactsize = Sb.st_size; + if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) { + syslog(L_FATAL, "%s active file %s is too large", LogName, + ICDactpath); + exit(1); + } + ICDactsize = (int) Sb.st_size; + ICDactpointer = mmap(NULL, ICDactsize, PROT_READ | PROT_WRITE, MAP_SHARED, ICDactfd, 0); if (ICDactpointer == (char *) -1) { @@ -465,11 +471,31 @@ ICDreadactive(char **endp) #else /* !HAVE_MMAP */ + /* Reject an already-oversized active file before asking the generic + reader to allocate it. ICDactsize below still comes from the exact + stat used to size and read the returned buffer. */ + if (fstat(ICDactfd, &Sb) < 0) { + syslog(L_FATAL, "%s cant fstat %d %s %m", LogName, ICDactfd, + ICDactpath); + exit(1); + } + if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) { + syslog(L_FATAL, "%s active file %s is too large", LogName, + ICDactpath); + exit(1); + } if ((ICDactpointer = ReadInDescriptor(ICDactfd, &Sb)) == NULL) { syslog(L_FATAL, "%s cant read %s %m", LogName, ICDactpath); exit(1); } - ICDactsize = Sb.st_size; + if (Sb.st_size < 0 || (uintmax_t) Sb.st_size > INT_MAX) { + free(ICDactpointer); + ICDactpointer = NULL; + syslog(L_FATAL, "%s active file %s is too large", LogName, + ICDactpath); + exit(1); + } + ICDactsize = (int) Sb.st_size; #endif /* HAVE_MMAP */ diff --git a/lib/buffer.c b/lib/buffer.c index ea8837fef..51a216411 100644 --- a/lib/buffer.c +++ b/lib/buffer.c @@ -319,6 +319,11 @@ buffer_read_file(struct buffer *buffer, int fd) if (fstat(fd, &st) < 0) return false; - buffer_resize(buffer, st.st_size + used); + if (st.st_size < 0 || used > SIZE_MAX - 1023 + || (uintmax_t) st.st_size > SIZE_MAX - used - 1023) { + errno = EOVERFLOW; + return false; + } + buffer_resize(buffer, (size_t) st.st_size + used); return buffer_read_all(buffer, fd); } diff --git a/lib/readin.c b/lib/readin.c index d04f72f46..4582b01fd 100644 --- a/lib/readin.c +++ b/lib/readin.c @@ -14,14 +14,26 @@ int xread(int fd, char *p, off_t i) { - int count; + ssize_t count; + size_t request; + + if (i < 0) { + errno = EINVAL; + return -1; + } for (; i; p += count, i -= count) { + request = (uintmax_t) i > (uintmax_t) SSIZE_MAX ? SSIZE_MAX + : (size_t) i; do { - count = read(fd, p, i); + count = read(fd, p, request); } while (count == -1 && errno == EINTR); - if (count <= 0) + if (count < 0) return -1; + if (count == 0) { + errno = EIO; + return -1; + } } return 0; } @@ -35,31 +47,32 @@ ReadInDescriptor(int fd, struct stat *Sbp) { struct stat mystat; char *p; + size_t size; int oerrno; if (Sbp == NULL) Sbp = &mystat; /* Get the size, and enough memory. */ - if (fstat(fd, Sbp) < 0) { - oerrno = errno; - close(fd); - errno = oerrno; + if (fstat(fd, Sbp) < 0) + return NULL; + if (Sbp->st_size < 0 || (uintmax_t) Sbp->st_size >= SIZE_MAX) { + errno = EOVERFLOW; return NULL; } - p = xmalloc(Sbp->st_size + 1); + size = (size_t) Sbp->st_size; + p = xmalloc(size + 1); /* Slurp, slurp. */ if (xread(fd, p, Sbp->st_size) < 0) { oerrno = errno; free(p); - close(fd); errno = oerrno; return NULL; } /* Terminate the string; terminate the routine. */ - p[Sbp->st_size] = '\0'; + p[size] = '\0'; return p; } @@ -72,12 +85,18 @@ char * ReadInFile(const char *name, struct stat *Sbp) { char *p; - int fd; + int fd, oerrno; if ((fd = open(name, O_RDONLY)) < 0) return NULL; p = ReadInDescriptor(fd, Sbp); - close(fd); + if (p == NULL) { + oerrno = errno; + close(fd); + errno = oerrno; + } else { + close(fd); + } return p; } diff --git a/lib/wire.c b/lib/wire.c index 8b09fadd7..73fa1d266 100644 --- a/lib/wire.c +++ b/lib/wire.c @@ -23,6 +23,7 @@ #include "portable/system.h" #include +#include #include "inn/libinn.h" #include "inn/wire.h" @@ -195,23 +196,36 @@ char * wire_from_native(const char *article, size_t len, size_t *newlen) { size_t bytes; + size_t extra; + size_t extra_limit; char *newart; const char *p; char *dest; bool at_start = true; + *newlen = 0; + if (len > SIZE_MAX - 4) { + errno = EOVERFLOW; + return NULL; + } + extra_limit = SIZE_MAX - 4 - len; + /* First go thru article and count number of bytes we need. Add a CR for every LF and an extra character for any period at the beginning of a - line for dot-stuffing. Add 3 characters at the end for .\r\n. */ - for (bytes = 0, p = article; p < article + len; p++) { + line for dot-stuffing. Each input byte adds at most one extra byte, + so extra cannot overflow. Add 3 characters at the end for .\r\n. */ + for (extra = 0, p = article; p < article + len; p++) { if (at_start && *p == '.') - bytes++; - bytes++; + extra++; at_start = (*p == '\n'); if (at_start) - bytes++; + extra++; + } + if (extra > extra_limit) { + errno = EOVERFLOW; + return NULL; } - bytes += 3; + bytes = len + extra + 3; /* Now copy the article, making the required changes. */ newart = xmalloc(bytes + 1); diff --git a/storage/buffindexed/buffindexed.c b/storage/buffindexed/buffindexed.c index 3e8262102..6d80ab95b 100644 --- a/storage/buffindexed/buffindexed.c +++ b/storage/buffindexed/buffindexed.c @@ -265,6 +265,7 @@ static int GROUPfd; static GROUPHEADER *GROUPheader = NULL; static GROUPENTRY *GROUPentries = NULL; static int GROUPcount = 0; +static size_t GROUPmapsize = 0; static GROUPLOC GROUPemptyloc = {-1}; #define NULLINDEX (-1) static OV ovnull = {0, NULLINDEX}; @@ -280,12 +281,14 @@ static OVSEARCH *Cachesearch; static int ovbuffmode; static GROUPLOC GROUPnewnode(void); -static bool GROUPremapifneeded(GROUPLOC loc); +static int GROUPmappingprot(int mode); +static bool GROUPremapifneeded(GROUPLOC loc, bool hash_locked); static void GROUPLOCclear(GROUPLOC *loc); static bool GROUPLOCempty(GROUPLOC loc); static bool GROUPlockhash(enum inn_locktype type); static bool GROUPlock(GROUPLOC gloc, enum inn_locktype type); -static off_t GROUPfilesize(int count); +static bool GROUPentrycount(off_t size, int *count); +static bool GROUPfilesize(int count, off_t *size); static bool GROUPexpand(int mode); static void *ovopensearch(const char *group, ARTNUM low, ARTNUM high, bool needov); @@ -1085,7 +1088,9 @@ buffindexed_open(int mode) { char *groupfn; struct stat sb; - int i, flag = 0; + off_t groupsize; + enum inn_locktype locktype; + int i, flag; static int uninitialized = 1; ULONG on, off; @@ -1141,28 +1146,47 @@ buffindexed_open(int mode) return false; } + locktype = (mode & OV_WRITE) ? INN_LOCK_WRITE : INN_LOCK_READ; + if (!GROUPlockhash(locktype)) { + syswarn("buffindexed: Could not lock %s", groupfn); + free(groupfn); + close(GROUPfd); + return false; + } + if (fstat(GROUPfd, &sb) < 0) { syswarn("buffindexed: Could not fstat %s", groupfn); + GROUPlockhash(INN_LOCK_UNLOCK); + free(groupfn); + close(GROUPfd); + return false; + } + if (sb.st_size < 0) { + errno = EOVERFLOW; + syswarn("buffindexed: invalid size for %s", groupfn); + GROUPlockhash(INN_LOCK_UNLOCK); free(groupfn); close(GROUPfd); return false; } if (sb.st_size > (off_t) sizeof(GROUPHEADER)) { - if (mode & OV_READ) - flag |= PROT_READ; - if (mode & OV_WRITE) { - /* - * Note: below mapping of groupheader won't work unless we have - * both PROT_READ and PROT_WRITE perms. - */ - flag |= PROT_WRITE | PROT_READ; + flag = GROUPmappingprot(mode); + if (!GROUPentrycount(sb.st_size, &GROUPcount) + || !GROUPfilesize(GROUPcount, &groupsize)) { + syswarn("buffindexed: invalid size for %s", groupfn); + GROUPlockhash(INN_LOCK_UNLOCK); + free(groupfn); + close(GROUPfd); + return false; } - GROUPcount = (sb.st_size - sizeof(GROUPHEADER)) / sizeof(GROUPENTRY); - GROUPheader = - mmap(0, GROUPfilesize(GROUPcount), flag, MAP_SHARED, GROUPfd, 0); + GROUPmapsize = (size_t) groupsize; + GROUPheader = mmap(0, GROUPmapsize, flag, MAP_SHARED, GROUPfd, 0); if (GROUPheader == MAP_FAILED) { syswarn("buffindexed: Could not mmap %s in buffindexed_open", groupfn); + GROUPheader = NULL; + GROUPmapsize = 0; + GROUPlockhash(INN_LOCK_UNLOCK); free(groupfn); close(GROUPfd); return false; @@ -1170,12 +1194,15 @@ buffindexed_open(int mode) GROUPentries = (void *) &GROUPheader[1]; } else { GROUPcount = 0; + GROUPmapsize = 0; if (!GROUPexpand(mode)) { + GROUPlockhash(INN_LOCK_UNLOCK); free(groupfn); close(GROUPfd); return false; } } + GROUPlockhash(INN_LOCK_UNLOCK); fdflag_close_exec(GROUPfd, true); free(groupfn); @@ -1185,19 +1212,49 @@ buffindexed_open(int mode) } static GROUPLOC -GROUPfind(const char *group, bool Ignoredeleted) +GROUPfind(const char *group, bool Ignoredeleted, bool *failed) { HASH grouphash; unsigned int i; + int steps = 0; GROUPLOC loc; grouphash = Hash(group, strlen(group)); memcpy(&i, &grouphash, sizeof(i)); + if (failed != NULL) + *failed = false; + if (GROUPheader == NULL || GROUPentries == NULL) { + if (failed != NULL) + *failed = true; + return GROUPemptyloc; + } loc = GROUPheader->hash[i % GROUPHEADERHASHSIZE]; - GROUPremapifneeded(loc); + if (!GROUPremapifneeded(loc, false)) { + if (failed != NULL) + *failed = true; + return GROUPemptyloc; + } + loc = GROUPheader->hash[i % GROUPHEADERHASHSIZE]; while (!GROUPLOCempty(loc)) { + if (loc.recno >= GROUPcount && !GROUPremapifneeded(loc, false)) { + if (failed != NULL) + *failed = true; + return GROUPemptyloc; + } + if (loc.recno >= GROUPcount) { + warn("buffindexed: group index entry %d out of range", loc.recno); + if (failed != NULL) + *failed = true; + return GROUPemptyloc; + } + if (steps++ >= GROUPcount) { + warn("buffindexed: cycle in group index chain"); + if (failed != NULL) + *failed = true; + return GROUPemptyloc; + } if (GROUPentries[loc.recno].deleted == 0 || Ignoredeleted) { if (memcmp(&grouphash, &GROUPentries[loc.recno].hash, sizeof(HASH)) == 0) { @@ -1215,7 +1272,7 @@ buffindexed_groupstats(const char *group, int *lo, int *hi, int *count, { GROUPLOC gloc; - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { return false; } @@ -1250,6 +1307,7 @@ setinitialge(GROUPENTRY *ge, HASH grouphash, char *flag, GROUPLOC next, bool buffindexed_groupadd(const char *group, ARTNUM lo, ARTNUM hi, char *flag) { + bool failed; unsigned int i; HASH grouphash; GROUPLOC gloc; @@ -1258,7 +1316,9 @@ buffindexed_groupadd(const char *group, ARTNUM lo, ARTNUM hi, char *flag) struct ov_name_table *ntp; #endif /* OV_DEBUG */ - gloc = GROUPfind(group, true); + gloc = GROUPfind(group, true, &failed); + if (failed) + return false; if (!GROUPLOCempty(gloc)) { ge = &GROUPentries[gloc.recno]; if (GROUPentries[gloc.recno].deleted != 0) { @@ -1272,8 +1332,13 @@ buffindexed_groupadd(const char *group, ARTNUM lo, ARTNUM hi, char *flag) grouphash = Hash(group, strlen(group)); memcpy(&i, &grouphash, sizeof(i)); i = i % GROUPHEADERHASHSIZE; - GROUPlockhash(INN_LOCK_WRITE); + if (!GROUPlockhash(INN_LOCK_WRITE)) + return false; gloc = GROUPnewnode(); + if (GROUPLOCempty(gloc)) { + GROUPlockhash(INN_LOCK_UNLOCK); + return false; + } ge = &GROUPentries[gloc.recno]; setinitialge(ge, grouphash, flag, GROUPheader->hash[i], lo, hi); GROUPheader->hash[i] = gloc; @@ -1293,46 +1358,145 @@ buffindexed_groupadd(const char *group, ARTNUM lo, ARTNUM hi, char *flag) return true; } -static off_t -GROUPfilesize(int count) +static bool +GROUPentrycount(off_t size, int *count) { - return ((off_t) count * sizeof(GROUPENTRY)) + sizeof(GROUPHEADER); + uintmax_t payload; + uintmax_t entries; + + if (size < (off_t) sizeof(GROUPHEADER)) { + errno = EINVAL; + return false; + } + if ((uintmax_t) size > SIZE_MAX) { + errno = EOVERFLOW; + return false; + } + payload = (uintmax_t) size - sizeof(GROUPHEADER); + if (payload % sizeof(GROUPENTRY) != 0) { + errno = EINVAL; + return false; + } + entries = payload / sizeof(GROUPENTRY); + if (entries > INT_MAX) { + errno = EOVERFLOW; + return false; + } + *count = (int) entries; + return true; +} + +static bool +GROUPfilesize(int count, off_t *size) +{ + uintmax_t bytes; + + if (count < 0 + || (uintmax_t) count + > (UINTMAX_MAX - sizeof(GROUPHEADER)) / sizeof(GROUPENTRY)) { + errno = EOVERFLOW; + return false; + } + bytes = sizeof(GROUPHEADER) + (uintmax_t) count * sizeof(GROUPENTRY); + if (bytes > SIZE_MAX) { + errno = EOVERFLOW; + return false; + } + *size = (off_t) bytes; + if (*size < 0 || (uintmax_t) *size != bytes) { + errno = EOVERFLOW; + return false; + } + return true; +} + +/* Return the mmap protection required by an overview open mode. Writers + * also need read permission because the mapped header is inspected. */ +static int +GROUPmappingprot(int mode) +{ + int prot = 0; + + if (mode & OV_READ) + prot |= PROT_READ; + if (mode & OV_WRITE) + prot |= PROT_READ | PROT_WRITE; + return prot; } /* Check if the given GROUPLOC refers to GROUPENTRY that we don't have mmap'ed, ** if so then see if the file has been grown by another writer and remmap */ static bool -GROUPremapifneeded(GROUPLOC loc) +GROUPremapifneeded(GROUPLOC loc, bool hash_locked) { + GROUPHEADER *newheader; struct stat sb; + off_t groupsize; + bool locked = false; + int count; - if (loc.recno < GROUPcount) + if (GROUPheader != NULL && GROUPentries != NULL + && loc.recno < GROUPcount) return true; - if (fstat(GROUPfd, &sb) < 0) + /* GROUPexpand publishes the new file size while holding this lock. A + remapper must not map that size until expansion has committed the new + header or left a stable zero-filled extension after failure. */ + if (!hash_locked) { + if (!GROUPlockhash(INN_LOCK_READ)) + return false; + locked = true; + } + + if (fstat(GROUPfd, &sb) < 0) { + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); return false; + } - if (GROUPfilesize(GROUPcount) >= sb.st_size) + if (GROUPheader != NULL && GROUPentries != NULL + && (uintmax_t) GROUPmapsize >= (uintmax_t) sb.st_size) { + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); return true; + } - if (GROUPheader) { - if (munmap((void *) GROUPheader, GROUPfilesize(GROUPcount)) < 0) { - syswarn("buffindexed: Could not munmap group.index in " - "GROUPremapifneeded"); - return false; - } + if (!GROUPentrycount(sb.st_size, &count) + || !GROUPfilesize(count, &groupsize)) { + syswarn("buffindexed: invalid group.index size"); + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); + return false; } - GROUPcount = (sb.st_size - sizeof(GROUPHEADER)) / sizeof(GROUPENTRY); - GROUPheader = mmap(0, GROUPfilesize(GROUPcount), PROT_READ | PROT_WRITE, - MAP_SHARED, GROUPfd, 0); - if (GROUPheader == MAP_FAILED) { + newheader = mmap(0, (size_t) groupsize, GROUPmappingprot(ovbuffmode), + MAP_SHARED, GROUPfd, 0); + if (newheader == MAP_FAILED) { syswarn( "buffindexed: Could not mmap group.index in GROUPremapifneeded"); + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); return false; } + + if (GROUPheader != NULL) { + if (munmap((void *) GROUPheader, GROUPmapsize) < 0) { + syswarn("buffindexed: Could not munmap group.index in " + "GROUPremapifneeded"); + munmap(newheader, (size_t) groupsize); + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); + return false; + } + } + + GROUPcount = count; + GROUPmapsize = (size_t) groupsize; + GROUPheader = newheader; GROUPentries = (void *) &GROUPheader[1]; + if (locked) + GROUPlockhash(INN_LOCK_UNLOCK); return true; } @@ -1341,36 +1505,52 @@ GROUPremapifneeded(GROUPLOC loc) static bool GROUPexpand(int mode) { + GROUPHEADER *newheader; + struct stat st; + off_t groupsize, oldsize; int i; - int flag = 0; + int newcount; - if (GROUPheader) { - if (munmap((void *) GROUPheader, GROUPfilesize(GROUPcount)) < 0) { - syswarn( - "buffindexed: Could not munmap group.index in GROUPexpand"); - return false; - } + if (GROUPcount > INT_MAX - 1024) { + errno = EOVERFLOW; + syswarn("buffindexed: group.index has too many entries"); + return false; } - GROUPcount += 1024; - if (ftruncate(GROUPfd, GROUPfilesize(GROUPcount)) < 0) { - syswarn("buffindexed: Could not extend group.index"); + newcount = GROUPcount + 1024; + if (!GROUPfilesize(newcount, &groupsize)) { + syswarn("buffindexed: invalid expanded group.index size"); return false; } - if (mode & OV_READ) - flag |= PROT_READ; - if (mode & OV_WRITE) { - /* - * Note: below check of magic won't work unless we have both PROT_READ - * and PROT_WRITE perms. - */ - flag |= PROT_WRITE | PROT_READ; + if (fstat(GROUPfd, &st) < 0) { + syswarn("buffindexed: Could not stat group.index before expansion"); + return false; } - GROUPheader = - mmap(0, GROUPfilesize(GROUPcount), flag, MAP_SHARED, GROUPfd, 0); - if (GROUPheader == MAP_FAILED) { + oldsize = st.st_size; + if (ftruncate(GROUPfd, groupsize) < 0) { + syswarn("buffindexed: Could not extend group.index"); + return false; + } + newheader = mmap(0, (size_t) groupsize, GROUPmappingprot(mode), MAP_SHARED, + GROUPfd, 0); + if (newheader == MAP_FAILED) { syswarn("buffindexed: Could not mmap group.index in GROUPexpand"); + /* If this is initial creation, restore the header-only file. For an + established mapping, keep the harmless zero-filled extension so + that a reader from an older process cannot be left mapped past EOF. + A later expansion attempt will initialize the same entries. */ + if (GROUPheader == NULL && ftruncate(GROUPfd, oldsize) < 0) + syswarn("buffindexed: Could not undo group.index expansion"); return false; } + if (GROUPheader != NULL + && munmap((void *) GROUPheader, GROUPmapsize) < 0) { + syswarn("buffindexed: Could not munmap group.index in GROUPexpand"); + munmap(newheader, (size_t) groupsize); + return false; + } + GROUPcount = newcount; + GROUPmapsize = (size_t) groupsize; + GROUPheader = newheader; GROUPentries = (void *) &GROUPheader[1]; if (GROUPheader->magic != GROUPHEADERMAGIC) { GROUPheader->magic = GROUPHEADERMAGIC; @@ -1392,25 +1572,41 @@ GROUPnewnode(void) { GROUPLOC loc; - /* If we didn't find any free space, then make some */ + /* If we didn't find any free space, first pick up an expansion performed + by another writer, and then make more space if necessary. */ + if (GROUPLOCempty(GROUPheader->freelist)) { + loc.recno = INT_MAX; + if (!GROUPremapifneeded(loc, true)) + return GROUPemptyloc; + } if (GROUPLOCempty(GROUPheader->freelist)) { if (!GROUPexpand(ovbuffmode)) { return GROUPemptyloc; } } - assert(!GROUPLOCempty(GROUPheader->freelist)); + + loc = GROUPheader->freelist; + if (!GROUPremapifneeded(loc, true)) + return GROUPemptyloc; loc = GROUPheader->freelist; - GROUPheader->freelist = GROUPentries[GROUPheader->freelist.recno].next; + if (GROUPLOCempty(loc) || loc.recno >= GROUPcount) { + warn("buffindexed: freelist entry %d out of range", loc.recno); + return GROUPemptyloc; + } + GROUPheader->freelist = GROUPentries[loc.recno].next; return loc; } bool buffindexed_groupdel(const char *group) { + bool failed; GROUPLOC gloc; GROUPENTRY *ge; - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, &failed); + if (failed) + return false; if (GROUPLOCempty(gloc)) { return true; } @@ -1685,6 +1881,7 @@ bool buffindexed_add(const char *group, ARTNUM artnum, TOKEN token, char *data, int len, time_t arrived, time_t expires) { + bool failed; GROUPLOC gloc; GROUPENTRY *ge; @@ -1693,7 +1890,9 @@ buffindexed_add(const char *group, ARTNUM artnum, TOKEN token, char *data, return true; } - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, &failed); + if (failed) + return false; if (GROUPLOCempty(gloc)) { return true; } @@ -1768,6 +1967,8 @@ ovgroupunmap(void) for (i = 0; i < GROUPDATAHASHSIZE; i++) { for (gdb = groupdatablock[i]; gdb != NULL; gdb = gdbnext) { gdbnext = gdb->next; + if (gdb->mmapped) + munmap(gdb->addr, gdb->len); free(gdb); } groupdatablock[i] = NULL; @@ -1788,6 +1989,23 @@ ovgroupunmap(void) } } +/* Failure must invalidate cached index state as well as temporary mappings. + A zero Gibcount with a non-NULL, partially filled Gib is not a valid cache + entry (gettoken expects at least one element whenever Gib is non-NULL). */ +static void +ovgroupunmap_failed(void) +{ + ovgroupunmap(); + free(Gib); + Gib = NULL; + Gibcount = 0; + if (Cachesearch != NULL) { + free(Cachesearch->group); + free(Cachesearch); + Cachesearch = NULL; + } +} + static void insertgdb(OV *ov, GROUPDATABLOCK *gdb) { @@ -1825,7 +2043,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) OV ov = ge->baseindex; OVBUFF *ovbuff; GROUPDATABLOCK *gdb; - int pagefudge, limit, i, capacity, count, len; + int pagefudge, limit, i, capacity, count, expected, len; off_t offset, mmapoffset; OVBLOCK *ovblock; void *addr; @@ -1835,14 +2053,15 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) Gibcount = 0; return true; } - Gibcount = ge->count; - if (Gibcount == 0) + expected = ge->count; + Gibcount = 0; + if (expected == 0) return true; - if (Gibcount < 0) { + if (expected < 0) { warn("buffindexed: negative overview count"); return false; } - capacity = Gibcount > OV_FUDGE ? OV_FUDGE : Gibcount; + capacity = expected > OV_FUDGE ? OV_FUDGE : expected; Gib = xmalloc(capacity * sizeof(OVINDEX)); count = 0; while (ov.index != NULLINDEX) { @@ -1850,14 +2069,14 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) if (giblist->ov.index == ov.index && giblist->ov.blocknum == ov.blocknum) { warn("buffindexed: loop in overview index blocks"); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } ovbuff = getovbuff(ov); if (ovbuff == NULL || !ovblockvalid(ovbuff, ov.blocknum)) { warn("buffindexed: invalid index block %d:%u", ov.index, ov.blocknum); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } offset = ovbuff->base + OV_OFFSET(ov.blocknum); @@ -1868,7 +2087,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) mmapoffset)) == MAP_FAILED) { syswarn("buffindexed: ovgroupmmap could not mmap index block"); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } ovblock = (void *) ((char *) addr + pagefudge); @@ -1878,7 +2097,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) if (limit < 0 || (size_t) limit > OVINDEXMAX) { warn("buffindexed: invalid index entry count %d", limit); munmap(addr, len); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } } else { @@ -1889,7 +2108,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) if (capacity > INT_MAX - OV_FUDGE) { warn("buffindexed: too many overview index entries"); munmap(addr, len); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } capacity += OV_FUDGE; @@ -1947,7 +2166,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) if (ovbuff == NULL || !ovblockvalid(ovbuff, ov.blocknum)) { warn("buffindexed: invalid overview data block %d:%u", ov.index, ov.blocknum); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } offset = ovbuff->base + OV_OFFSET(ov.blocknum); @@ -1958,8 +2177,7 @@ ovgroupmmap(GROUPENTRY *ge, ARTNUM low, ARTNUM high, bool needov) ovbuff->fd, mmapoffset)) == MAP_FAILED) { syswarn("buffindexed: ovgroupmmap could not mmap data block"); - free(gdb); - ovgroupunmap(); + ovgroupunmap_failed(); return false; } gdb->data = (char *) gdb->addr + pagefudge; @@ -1976,7 +2194,7 @@ ovopensearch(const char *group, ARTNUM low, ARTNUM high, bool needov) GROUPENTRY *ge; OVSEARCH *search; - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) return NULL; @@ -2017,7 +2235,7 @@ buffindexed_opensearch(const char *group, int low, int high) Cachesearch = NULL; } } - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { return NULL; } @@ -2150,24 +2368,16 @@ static void ovclosesearch(void *handle, bool freeblock) { OVSEARCH *search = (OVSEARCH *) handle; - GROUPDATABLOCK *gdb; - int i; #ifdef OV_DEBUG GROUPENTRY *ge; GROUPLOC gloc; #endif /* OV_DEBUG */ - for (i = 0; i < GROUPDATAHASHSIZE; i++) { - for (gdb = groupdatablock[i]; gdb != NULL; gdb = gdb->next) { - if (gdb->mmapped) - munmap(gdb->addr, gdb->len); - } - } if (search->gdb.mmapped) munmap(search->gdb.addr, search->gdb.len); if (freeblock) { #ifdef OV_DEBUG - gloc = GROUPfind(search->group, false); + gloc = GROUPfind(search->group, false, NULL); if (!GROUPLOCempty(gloc)) { ge = &GROUPentries[gloc.recno]; freegroupblock(ge); @@ -2202,6 +2412,9 @@ static bool gettoken(ARTNUM artnum, TOKEN *token) { int i, j, offset, limit; + + if (Gib == NULL || Gibcount <= 0) + return false; offset = 0; limit = Gibcount; for (i = (limit - offset) / 2; i > 0; i = (limit - offset) / 2) { @@ -2264,7 +2477,7 @@ buffindexed_getartinfo(const char *group, ARTNUM artnum, TOKEN *token) return true; else { /* examine to see if overview index are increased */ - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { return false; } @@ -2288,7 +2501,7 @@ buffindexed_getartinfo(const char *group, ARTNUM artnum, TOKEN *token) } } } - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { return false; } @@ -2325,7 +2538,13 @@ buffindexed_expiregroup(const char *group, int *lo, struct history *h) OVSEARCH search = {0}; if (group == NULL) { - for (i = 0; i < GROUPheader->freelist.recno; i++) { + /* Refresh a mapping that may have been extended by another writer, + then inspect every slot. Empty slots have count zero, while -1 in + the freelist means all slots have been allocated. */ + gloc.recno = INT_MAX; + if (!GROUPremapifneeded(gloc, false)) + return false; + for (i = 0; i < GROUPcount; i++) { gloc.recno = i; GROUPlock(gloc, INN_LOCK_WRITE); ge = &GROUPentries[gloc.recno]; @@ -2364,7 +2583,7 @@ buffindexed_expiregroup(const char *group, int *lo, struct history *h) } return true; } - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { return false; } @@ -2604,12 +2823,14 @@ buffindexed_close(void) close(GROUPfd); if (GROUPheader) { - if (munmap((void *) GROUPheader, GROUPfilesize(GROUPcount)) < 0) { + if (munmap((void *) GROUPheader, GROUPmapsize) < 0) { syswarn("buffindexed: could not munmap group.index in " "buffindexed_close"); return; } GROUPheader = NULL; + GROUPentries = NULL; + GROUPmapsize = 0; } /* sync the bit field */ @@ -2710,7 +2931,7 @@ main(int argc, char **argv) GROUPlock(gloc, INN_LOCK_UNLOCK); exit(0); } - gloc = GROUPfind(group, false); + gloc = GROUPfind(group, false, NULL); if (GROUPLOCempty(gloc)) { fprintf(stderr, "gloc is null\n"); } diff --git a/storage/timecaf/caf.c b/storage/timecaf/caf.c index ddb0ce4cb..1b36e6f4c 100644 --- a/storage/timecaf/caf.c +++ b/storage/timecaf/caf.c @@ -21,6 +21,9 @@ #define CAF_INNARDS 1 #include "caf.h" +/* Define this instead of littering bitmap formulas with semi-mysterious 8s. */ +#define BYTEWIDTH 8 + /* following code lifted from inndf.c */ #ifdef HAVE_STATVFS @@ -96,17 +99,25 @@ CAFError(int code) static int OurRead(int fd, void *buf, size_t n) { + char *p = buf; ssize_t rval; - - rval = read(fd, buf, n); - if (rval < 0) { - CAFError(CAF_ERR_IO); - return -1; - } - if ((size_t) rval < n) { - /* not enough data! */ - CAFError(CAF_ERR_BADFILE); - return -1; + size_t request; + + while (n > 0) { + request = n > (size_t) SSIZE_MAX ? (size_t) SSIZE_MAX : n; + do { + rval = read(fd, p, request); + } while (rval < 0 && errno == EINTR); + if (rval < 0) { + CAFError(CAF_ERR_IO); + return -1; + } + if (rval == 0) { + CAFError(CAF_ERR_BADFILE); + return -1; + } + p += rval; + n -= (size_t) rval; } return 0; } @@ -115,15 +126,7 @@ OurRead(int fd, void *buf, size_t n) static int OurWrite(int fd, const void *buf, size_t n) { - ssize_t rval; - - rval = write(fd, buf, n); - if (rval < 0) { - CAFError(CAF_ERR_IO); - return -1; - } - if ((size_t) rval < n) { - /* not enough data written */ + if (xwrite(fd, buf, n) < 0) { CAFError(CAF_ERR_IO); return -1; } @@ -150,9 +153,74 @@ CAFReadHeader(int fd, CAFHEADER *h) CAFError(CAF_ERR_BADFILE); return -1; } + /* BlockSize was zero in some old CAF headers, where it meant the + default. Normalize it before validating or using bitmap arithmetic. */ + if (h->BlockSize == 0) + h->BlockSize = CAF_DEFAULT_BLOCKSIZE; return 0; } +/* Validate the variable-sized bitmap and TOC fields from a CAF header and + return the TOC values in types suitable for allocation and positioning. + The output pointers are optional. */ +static bool +CAFGetTOCInfo(const CAFHEADER *head, size_t *countp, size_t *bytesp, + off_t *offsetp) +{ + ARTNUM span; + size_t bitmap_bytes, count, offset; + uintmax_t reserved_bytes, toc_end; + off_t file_offset; + + if (head->BlockSize < sizeof(CAFHEADER) + || head->FreeZoneIndexSize + != head->BlockSize - sizeof(CAFHEADER) + || head->FreeZoneIndexSize > UINT_MAX / BYTEWIDTH + || SIZE_MAX / head->BlockSize / head->BlockSize < BYTEWIDTH) + return false; + if (head->FreeZoneIndexSize + > (SIZE_MAX - head->FreeZoneIndexSize) + / (head->BlockSize * (size_t) BYTEWIDTH)) + return false; + bitmap_bytes = + head->FreeZoneIndexSize + + head->BlockSize * head->FreeZoneIndexSize * (size_t) BYTEWIDTH; + if (head->FreeZoneTabSize != bitmap_bytes) + return false; + + if (head->High < head->Low || head->NumSlots == 0) + return false; + span = head->High - head->Low; + if (span >= head->NumSlots + || (uintmax_t) head->NumSlots > SIZE_MAX / sizeof(CAFTOCENT)) + return false; + + if (head->FreeZoneTabSize > SIZE_MAX - sizeof(CAFHEADER)) + return false; + offset = sizeof(CAFHEADER) + head->FreeZoneTabSize; + file_offset = (off_t) offset; + if (file_offset < 0 || (uintmax_t) file_offset != (uintmax_t) offset) + return false; + + reserved_bytes = (uintmax_t) head->NumSlots * sizeof(CAFTOCENT); + if ((uintmax_t) offset > UINTMAX_MAX - reserved_bytes) + return false; + toc_end = (uintmax_t) offset + reserved_bytes; + if (head->StartDataBlock < 0 + || toc_end > (uintmax_t) head->StartDataBlock + || head->StartDataBlock % head->BlockSize != 0) + return false; + + count = (size_t) span + 1; + if (countp != NULL) + *countp = count; + if (bytesp != NULL) + *bytesp = count * sizeof(CAFTOCENT); + if (offsetp != NULL) + *offsetp = file_offset; + return true; +} + /* ** Seek to the TOC entry for a given article. As usual, -1 for error, 0 succ. */ @@ -160,10 +228,19 @@ CAFReadHeader(int fd, CAFHEADER *h) static int CAFSeekTOCEnt(int fd, CAFHEADER *head, ARTNUM art) { + ARTNUM slot; off_t offset; - offset = sizeof(CAFHEADER) + head->FreeZoneTabSize; - offset += (art - head->Low) * sizeof(CAFTOCENT); + if (!CAFGetTOCInfo(head, NULL, NULL, &offset) || art < head->Low) { + CAFError(CAF_ERR_BADFILE); + return -1; + } + slot = art - head->Low; + if (slot >= head->NumSlots) { + CAFError(CAF_ERR_BADFILE); + return -1; + } + offset += (off_t) ((uintmax_t) slot * sizeof(CAFTOCENT)); if (lseek(fd, offset, SEEK_SET) < 0) { CAFError(CAF_ERR_IO); return -1; @@ -232,17 +309,19 @@ CAFDisposeBitmap(CAFBITMAP *bm) ** Read the index bitmap from a CAF file, return a CAFBITMAP structure. */ -/* Define this instead of littering all our formulas with semi-mysterious 8s. - */ -#define BYTEWIDTH 8 - CAFBITMAP * CAFReadFreeBM(int fd, CAFHEADER *h) { size_t i; struct stat statbuf; CAFBITMAP *bm; + off_t max_offset; + uintmax_t max_data; + if (!CAFGetTOCInfo(h, NULL, NULL, NULL)) { + CAFError(CAF_ERR_BADFILE); + return NULL; + } if (lseek(fd, sizeof(CAFHEADER), SEEK_SET) < 0) { CAFError(CAF_ERR_IO); return NULL; @@ -251,8 +330,9 @@ CAFReadFreeBM(int fd, CAFHEADER *h) bm->FreeZoneTabSize = h->FreeZoneTabSize; bm->FreeZoneIndexSize = h->FreeZoneIndexSize; - bm->NumBMB = BYTEWIDTH * bm->FreeZoneIndexSize; - bm->BytesPerBMB = (h->BlockSize) * (h->BlockSize * BYTEWIDTH); + bm->NumBMB = (unsigned int) (BYTEWIDTH * bm->FreeZoneIndexSize); + bm->BytesPerBMB = + (size_t) h->BlockSize * h->BlockSize * (size_t) BYTEWIDTH; bm->BlockSize = h->BlockSize; bm->Blocks = xmalloc(bm->NumBMB * sizeof(CAFBMB *)); @@ -274,9 +354,27 @@ CAFReadFreeBM(int fd, CAFHEADER *h) CAFDisposeBitmap(bm); return NULL; } - /* round st_size down to a mult. of BlockSize */ - bm->MaxDataBlock = - (statbuf.st_size / bm->BlockSize) * bm->BlockSize + bm->BlockSize; + /* Round st_size down to a multiple of BlockSize and then point at the + following block, rejecting an off_t wrap at the top of the file. */ + if (statbuf.st_size < 0) { + CAFError(CAF_ERR_BADFILE); + CAFDisposeBitmap(bm); + return NULL; + } + max_data = ((uintmax_t) statbuf.st_size / bm->BlockSize) * bm->BlockSize; + if (max_data > UINTMAX_MAX - bm->BlockSize) { + CAFError(CAF_ERR_BADFILE); + CAFDisposeBitmap(bm); + return NULL; + } + max_data += bm->BlockSize; + max_offset = (off_t) max_data; + if (max_offset < 0 || (uintmax_t) max_offset != max_data) { + CAFError(CAF_ERR_BADFILE); + CAFDisposeBitmap(bm); + return NULL; + } + bm->MaxDataBlock = max_offset; /* (note: MaxDataBlock points to the block *after* the last block of the * file. */ return bm; @@ -287,10 +385,30 @@ CAFReadFreeBM(int fd, CAFHEADER *h) ** the new BMB appropriately. ** Return NULL on failure, and the BMB * on success. */ +static bool +CAFBMBOffset(unsigned int blkno, const CAFBITMAP *bm, off_t *offset) +{ + uintmax_t block, bytes; + + block = (uintmax_t) blkno + 1; + if (bm->BlockSize == 0 || block > UINTMAX_MAX / bm->BlockSize) { + errno = EOVERFLOW; + return false; + } + bytes = block * bm->BlockSize; + *offset = (off_t) bytes; + if (*offset < 0 || (uintmax_t) *offset != bytes) { + errno = EOVERFLOW; + return false; + } + return true; +} + static CAFBMB * CAFFetchBMB(unsigned int blkno, int fd, CAFBITMAP *bm) { CAFBMB *newbmb; + off_t offset; ASSERT(blkno < bm->NumBMB); /* if already in memory, don't need to do anything. */ @@ -310,7 +428,13 @@ CAFFetchBMB(unsigned int blkno, int fd, CAFBITMAP *bm) newbmb->BMBBits = xmalloc(bm->BlockSize); - if (lseek(fd, (blkno + 1) * bm->BlockSize, SEEK_SET) < 0) { + if (!CAFBMBOffset(blkno, bm, &offset)) { + free(newbmb->BMBBits); + free(newbmb); + CAFError(CAF_ERR_BADFILE); + return NULL; + } + if (lseek(fd, offset, SEEK_SET) < 0) { free(newbmb->BMBBits); free(newbmb); CAFError(CAF_ERR_IO); @@ -335,6 +459,7 @@ static int CAFFlushBMB(unsigned int blkno, int fd, CAFBITMAP *bm) { CAFBMB *bmb; + off_t offset; ASSERT(blkno < bm->NumBMB); @@ -345,7 +470,11 @@ CAFFlushBMB(unsigned int blkno, int fd, CAFBITMAP *bm) if (!bmb->Dirty) return 0; - if (lseek(fd, (blkno + 1) * bm->BlockSize, SEEK_SET) < 0) { + if (!CAFBMBOffset(blkno, bm, &offset)) { + CAFError(CAF_ERR_BADFILE); + return -1; + } + if (lseek(fd, offset, SEEK_SET) < 0) { CAFError(CAF_ERR_IO); return -1; } @@ -607,6 +736,11 @@ CAFOpenArtRead(const char *path, ARTNUM art, size_t *len) close(fd); return -1; } + if (!CAFGetTOCInfo(&head, NULL, NULL, NULL)) { + CAFError(CAF_ERR_BADFILE); + close(fd); + return -1; + } /* Is the requested article even in the file? */ if (art < head.Low || art > head.High) { @@ -642,8 +776,9 @@ CAFOpenArtRead(const char *path, ARTNUM art, size_t *len) return -1; } if (st.st_size < 0 || tocent.Offset < 0 - || tocent.Size > (size_t) st.st_size - || (size_t) tocent.Offset > (size_t) st.st_size - tocent.Size) { + || (uintmax_t) tocent.Size > (uintmax_t) st.st_size + || (uintmax_t) tocent.Offset + > (uintmax_t) st.st_size - tocent.Size) { CAFError(CAF_ERR_BADFILE); close(fd); return -1; @@ -706,12 +841,19 @@ CAFCreateCAFFile(char *cfpath, ARTNUM artnum, ARTNUM tocsize, size_t estcfsize, int nolink, char *temppath, size_t pathlen) { CAFHEADER head; - int fd; + int fd, oerrno; char path[SPOOLNAMEBUFF]; char finalpath[SPOOLNAMEBUFF]; off_t offset; + uintmax_t rounded, table_end, toc_bytes; char nulls[1]; + if (tocsize == 0 + || (uintmax_t) tocsize > SIZE_MAX / sizeof(CAFTOCENT)) { + errno = EOVERFLOW; + CAFError(CAF_ERR_IO); + return -1; + } strlcpy(finalpath, cfpath, sizeof(finalpath)); /* create path with PID attached */ snprintf(path, sizeof(path), "%s.%lu", cfpath, (unsigned long) getpid()); @@ -741,23 +883,42 @@ CAFCreateCAFFile(char *cfpath, ARTNUM artnum, ARTNUM tocsize, size_t estcfsize, head.FreeZoneTabSize = head.FreeZoneIndexSize + head.BlockSize * head.FreeZoneIndexSize * BYTEWIDTH; - head.StartDataBlock = CAFRoundOffsetUp( - sizeof(CAFHEADER) + head.FreeZoneTabSize + tocsize * sizeof(CAFTOCENT), - head.BlockSize); + toc_bytes = (uintmax_t) tocsize * sizeof(CAFTOCENT); + if ((uintmax_t) head.FreeZoneTabSize + > UINTMAX_MAX - sizeof(CAFHEADER) + || toc_bytes + > UINTMAX_MAX - sizeof(CAFHEADER) - head.FreeZoneTabSize) { + errno = EOVERFLOW; + CAFError(CAF_ERR_IO); + goto fail; + } + table_end = sizeof(CAFHEADER) + head.FreeZoneTabSize + toc_bytes; + if (table_end > UINTMAX_MAX - (head.BlockSize - 1)) { + errno = EOVERFLOW; + CAFError(CAF_ERR_IO); + goto fail; + } + rounded = ((table_end + head.BlockSize - 1) / head.BlockSize) + * head.BlockSize; + head.StartDataBlock = (off_t) rounded; + offset = (off_t) table_end; + if (head.StartDataBlock < 0 + || (uintmax_t) head.StartDataBlock != rounded || offset < 0 + || (uintmax_t) offset != table_end) { + errno = EOVERFLOW; + CAFError(CAF_ERR_IO); + goto fail; + } head.spare[0] = head.spare[1] = head.spare[2] = 0; if (OurWrite(fd, &head, sizeof(head)) < 0) { - close(fd); - return -1; + goto fail; } - offset = - sizeof(CAFHEADER) + head.FreeZoneTabSize + sizeof(CAFTOCENT) * tocsize; - if (lseek(fd, offset, SEEK_SET) < 0) { CAFError(CAF_ERR_IO); - return -1; + goto fail; } /* ** put a null after the TOC as a 'placeholder', so that we'll have a sparse @@ -765,15 +926,13 @@ CAFCreateCAFFile(char *cfpath, ARTNUM artnum, ARTNUM tocsize, size_t estcfsize, */ nulls[0] = 0; if (OurWrite(fd, nulls, 1) < 0) { - close(fd); - return -1; + goto fail; } /* shouldn't be anyone else locking our file, since temp file has unique PID-based name ... */ if (!inn_lock_file(fd, INN_LOCK_WRITE, false)) { CAFError(CAF_ERR_IO); - close(fd); - return -1; + goto fail; } if (nolink) { @@ -789,11 +948,7 @@ CAFCreateCAFFile(char *cfpath, ARTNUM artnum, ARTNUM tocsize, size_t estcfsize, */ if (link(path, finalpath) < 0) { CAFError(CAF_ERR_IO); - /* bounced on the link attempt, go ahead and unlink the temp file and - * return. */ - unlink(path); - close(fd); - return -1; + goto fail; } /* ** Unlink the temp. link. Do we really care if this fails? XXX @@ -801,6 +956,13 @@ CAFCreateCAFFile(char *cfpath, ARTNUM artnum, ARTNUM tocsize, size_t estcfsize, */ unlink(path); return fd; + +fail: + oerrno = errno; + close(fd); + unlink(path); + errno = oerrno; + return -1; } /* @@ -900,6 +1062,11 @@ CAFStartWriteFd(int fd, ARTNUM *artp, size_t size) close(fd); return -1; } + if (!CAFGetTOCInfo(&head, NULL, NULL, NULL)) { + CAFError(CAF_ERR_BADFILE); + close(fd); + return -1; + } /* check for zero article number and handle accordingly. */ art = *artp; @@ -911,7 +1078,7 @@ CAFStartWriteFd(int fd, ARTNUM *artp, size_t size) } /* Is the requested article even in the file? */ - if (art < head.Low || art >= head.Low + head.NumSlots) { + if (art < head.Low || art - head.Low >= head.NumSlots) { CAFError(CAF_ERR_ARTWONTFIT); close(fd); return -1; @@ -1159,6 +1326,7 @@ CAFOpenReadTOC(char *path, CAFHEADER *ch, CAFTOCENT **tocpp) size_t count, nb; CAFTOCENT *tocp; off_t offset; + struct stat st; if ((fd = open(path, O_RDONLY)) < 0) { /* @@ -1180,19 +1348,24 @@ CAFOpenReadTOC(char *path, CAFHEADER *ch, CAFTOCENT **tocpp) } /* Allocate memory for TOC. */ - if (ch->High < ch->Low || ch->High - ch->Low >= ch->NumSlots - || ch->High - ch->Low + 1 > SIZE_MAX / sizeof(CAFTOCENT)) { + if (!CAFGetTOCInfo(ch, &count, &nb, &offset)) { + CAFError(CAF_ERR_BADFILE); + close(fd); + return -1; + } + if (fstat(fd, &st) < 0) { + CAFError(CAF_ERR_IO); + close(fd); + return -1; + } + if (st.st_size < 0 + || (uintmax_t) offset + nb > (uintmax_t) st.st_size) { CAFError(CAF_ERR_BADFILE); close(fd); return -1; } - count = ch->High - ch->Low + 1; - nb = sizeof(CAFTOCENT) * count; tocp = xmalloc(nb); - /* seek to beginning of TOC */ - offset = sizeof(CAFHEADER) + ch->FreeZoneTabSize; - if (lseek(fd, offset, SEEK_SET) < 0) { CAFError(CAF_ERR_IO); free(tocp); @@ -1223,11 +1396,13 @@ int CAFRemoveMultArts(char *path, unsigned int narts, ARTNUM *artnums) { int fd; + struct stat statbuf; CAFHEADER head; CAFTOCENT tocent; CAFBITMAP *freebitmap; ARTNUM art; - unsigned int numblksfreed, i, j; + size_t freed_bytes, i, numblksfreed; + unsigned int j; off_t curblk; int errorfound = false; @@ -1262,6 +1437,22 @@ CAFRemoveMultArts(char *path, unsigned int narts, ARTNUM *artnums) close(fd); return -1; } + if (!CAFGetTOCInfo(&head, NULL, NULL, NULL)) { + CAFError(CAF_ERR_BADFILE); + close(fd); + return -1; + } + if (fstat(fd, &statbuf) < 0) { + CAFError(CAF_ERR_IO); + close(fd); + return -1; + } + if (statbuf.st_size < 0 + || (uintmax_t) head.StartDataBlock > (uintmax_t) statbuf.st_size) { + CAFError(CAF_ERR_BADFILE); + close(fd); + return -1; + } if ((freebitmap = CAFReadFreeBM(fd, &head)) == NULL) { close(fd); @@ -1292,7 +1483,31 @@ CAFRemoveMultArts(char *path, unsigned int narts, ARTNUM *artnums) missing */ } - numblksfreed = (tocent.Size + head.BlockSize - 1) / head.BlockSize; + if (tocent.Offset < head.StartDataBlock + || (uintmax_t) tocent.Size > (uintmax_t) statbuf.st_size + || (uintmax_t) tocent.Offset + > (uintmax_t) statbuf.st_size - tocent.Size) { + CAFError(CAF_ERR_BADFILE); + close(fd); + CAFDisposeBitmap(freebitmap); + return -1; + } + numblksfreed = tocent.Size / head.BlockSize; + if (tocent.Size % head.BlockSize != 0) + numblksfreed++; + if (numblksfreed > SIZE_MAX / head.BlockSize) { + CAFError(CAF_ERR_BADFILE); + close(fd); + CAFDisposeBitmap(freebitmap); + return -1; + } + freed_bytes = numblksfreed * head.BlockSize; + if (head.Free > SIZE_MAX - freed_bytes) { + CAFError(CAF_ERR_BADFILE); + close(fd); + CAFDisposeBitmap(freebitmap); + return -1; + } /* Mark all the blocks as free. */ for (curblk = tocent.Offset, i = 0; i < numblksfreed; @@ -1300,7 +1515,7 @@ CAFRemoveMultArts(char *path, unsigned int narts, ARTNUM *artnums) CAFSetBlockFree(freebitmap, fd, curblk, 1); } /* Note the amount of free space added. */ - head.Free += numblksfreed * head.BlockSize; + head.Free += freed_bytes; /* and mark the tocent as a deleted entry. */ tocent.Size = 0; @@ -1434,24 +1649,22 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) size_t pathlen; CAFHEADER head, newhead; int fdin, fdout; - ARTNUM newlow; - ARTNUM i; + ARTNUM newlow, newtocsize; CAFTOCENT *tocarray, *tocp; CAFTOCENT *newtocarray, *newtocp; - size_t newtocsize; + size_t active_count, toc_bytes, toc_count, toc_index; FILE *infile, *outfile; - off_t startoffset, newstartoffset; + off_t datasize, newstartoffset, startoffset, toc_offset; char buf[BUFSIZ]; - int nbytes; - size_t ncur; - int n; + size_t n, nbytes, ncur; unsigned int blocksize; char *zerobuff; struct stat statbuf; - size_t datasize; + size_t estimated_size; double percentfree; int toc_needs_expansion; int toc_needs_compacting; + bool found_article, invalid_data_region, invalid_free_counter; #ifdef STATFUNCT struct STATSTRUC fsinfo; @@ -1470,8 +1683,10 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) */ if (errno != ENOENT) { CAFError(CAF_ERR_IO); + free(newpath); return -1; } else { + free(newpath); return 0; } } @@ -1493,6 +1708,7 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) /* Fetch the header */ if (CAFReadHeader(fdin, &head) < 0) { close(fdin); + free(newpath); return -1; } @@ -1501,14 +1717,37 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) close(fdin); CAFError(CAF_ERR_IO); perror(path); + free(newpath); + return -1; + } + if (!CAFGetTOCInfo(&head, &toc_count, &toc_bytes, &toc_offset) + || statbuf.st_size < 0 + || (uintmax_t) toc_offset > UINTMAX_MAX - toc_bytes + || (uintmax_t) toc_offset + toc_bytes + > (uintmax_t) statbuf.st_size) { + close(fdin); + CAFError(CAF_ERR_BADFILE); + free(newpath); return -1; } - /* compute amount of actual data in file. */ - datasize = statbuf.st_size - head.StartDataBlock; - if (datasize == 0) { + /* Defer rejecting a damaged data boundary until after checking whether + the TOC is empty. An empty orphan can still be safely unlinked. */ + invalid_data_region = + (uintmax_t) head.StartDataBlock > (uintmax_t) statbuf.st_size; + datasize = invalid_data_region ? 0 + : statbuf.st_size - head.StartDataBlock; + invalid_free_counter = + !invalid_data_region + && (uintmax_t) head.Free > (uintmax_t) datasize; + if (invalid_data_region || datasize == 0) { /* nothing in the file, set percentfree==0 so won't bother cleaning */ percentfree = 0; + } else if (invalid_free_counter) { + /* The old cleaner recovered from an inflated Free counter by doing a + full clean. Keep that behavior, but validate every copied article + before rewriting the header with Free reset to zero. */ + percentfree = 100.0; } else { percentfree = (100.0 * (double) head.Free) / (double) datasize; } @@ -1518,59 +1757,62 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) ** we can decide if a clean or a compaction is needed. */ - lseek(fdin, 0L, SEEK_SET); - /* make input file stdio-buffered. */ if ((infile = fdopen(fdin, "r+")) == NULL) { CAFError(CAF_ERR_IO); close(fdin); + free(newpath); return -1; } - /* Allocate memory for TOC. */ - tocarray = xmalloc((head.High - head.Low + 1) * sizeof(CAFTOCENT)); - - fseeko(infile, (off_t) (sizeof(CAFHEADER) + head.FreeZoneTabSize), - SEEK_SET); - - n = fread(tocarray, sizeof(CAFTOCENT), (head.High - head.Low + 1), infile); - if (n < 0) { + /* Allocate memory for and read the TOC. */ + tocarray = xmalloc(toc_bytes); + if (fseeko(infile, toc_offset, SEEK_SET) < 0) { CAFError(CAF_ERR_IO); fclose(infile); free(tocarray); free(newpath); return -1; } - - if ((unsigned long) n < (head.High - head.Low + 1)) { - CAFError(CAF_ERR_BADFILE); + n = fread(tocarray, sizeof(CAFTOCENT), toc_count, infile); + if (n != toc_count) { + CAFError(ferror(infile) ? CAF_ERR_IO : CAF_ERR_BADFILE); fclose(infile); free(tocarray); free(newpath); return -1; } - /* Scan to see what the new lower bound for CAF file should be. */ - newlow = head.High + 1; - - for (tocp = tocarray, i = head.Low; i <= head.High; ++tocp, ++i) { + /* Find the new lower bound. Validate article spans only if we actually + clean the file; compaction and the no-op path do not dereference them, + and must continue to tolerate a torn entry left by a crash. */ + found_article = false; + newlow = head.Low; + for (toc_index = 0; toc_index < toc_count; toc_index++) { + tocp = &tocarray[toc_index]; if (tocp->Size != 0) { - newlow = i; + newlow = head.Low + toc_index; + found_article = true; break; } } - /* - ** if newlow is head.High+1, the TOC is completely empty and we can - ** just remove the entire file. - */ - if (newlow == head.High + 1) { + /* If the TOC is completely empty, remove the entire file. */ + if (!found_article) { unlink(path); fclose(infile); free(tocarray); free(newpath); return 0; } + if (invalid_data_region) { + CAFError(CAF_ERR_BADFILE); + fclose(infile); + free(tocarray); + free(newpath); + return -1; + } + active_count = (size_t) (head.High - newlow) + 1; /* ** Ah. NOW we get to decide if we need a clean! @@ -1589,16 +1831,16 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) } toc_needs_compacting = 0; - if ((head.Low + head.NumSlots - head.NumSlots / TOC_COMPACT_RATIO) - <= head.High) { + if (head.High - head.Low + >= head.NumSlots - head.NumSlots / TOC_COMPACT_RATIO) { toc_needs_compacting = 1; } - if ((percentfree < PercentFreeThreshold) && (!toc_needs_expansion)) { + if (!invalid_free_counter && (percentfree < PercentFreeThreshold) + && (!toc_needs_expansion)) { /* no cleaning, but do we need a TOC compaction ? */ if (toc_needs_compacting) { - int delta; - CAFTOCENT *tocp2; + size_t delta; if (verbose) { printf("Compacting %s: Free=%lu (%f%%)\n", path, @@ -1608,18 +1850,16 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) delta = newlow - head.Low; /* slide TOC array down delta units. */ - for (i = newlow, tocp = tocarray, tocp2 = tocarray + delta; - i <= head.High; ++i) { - *tocp++ = *tocp2++; - } + memmove(tocarray, tocarray + delta, + active_count * sizeof(CAFTOCENT)); head.Low = newlow; /* note we don't set LastCleaned, this doesn't count a a clean. */ /* (XXX: do we need a LastCompacted as well? might be nice.) */ /* write new header on top of old */ - fseeko(infile, 0, SEEK_SET); - if (fwrite(&head, sizeof(CAFHEADER), 1, infile) < 1) { + if (fseeko(infile, 0, SEEK_SET) < 0 + || fwrite(&head, sizeof(CAFHEADER), 1, infile) < 1) { CAFError(CAF_ERR_IO); free(tocarray); free(newpath); @@ -1630,18 +1870,15 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) ** this next fseeko might actually fail, because we have buffered ** stuff that might fail on write. */ - if (fseeko(infile, sizeof(CAFHEADER) + head.FreeZoneTabSize, - SEEK_SET) - < 0) { + if (fseeko(infile, toc_offset, SEEK_SET) < 0) { perror(path); free(tocarray); free(newpath); fclose(infile); return -1; } - if (fwrite(tocarray, sizeof(CAFTOCENT), head.High - newlow + 1, - infile) - < head.High - newlow + 1 + if (fwrite(tocarray, sizeof(CAFTOCENT), active_count, infile) + < active_count || fflush(infile) < 0) { CAFError(CAF_ERR_IO); free(tocarray); @@ -1667,6 +1904,23 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) } } + /* A full clean copies article data, so reject entries that point outside + the data region rather than seeking through corrupt metadata. */ + for (toc_index = newlow - head.Low; toc_index < toc_count; toc_index++) { + tocp = &tocarray[toc_index]; + if (tocp->Size != 0 + && (tocp->Offset < head.StartDataBlock + || (uintmax_t) tocp->Size > (uintmax_t) statbuf.st_size + || (uintmax_t) tocp->Offset + > (uintmax_t) statbuf.st_size - tocp->Size)) { + CAFError(CAF_ERR_BADFILE); + fclose(infile); + free(tocarray); + free(newpath); + return -1; + } + } + /* ** If OS supports it, try to check for free space and skip this file if ** not enough free space on this filesystem. @@ -1683,8 +1937,10 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) (CAFRoundOffsetUp((n), fsinfo.STATMULTI) / fsinfo.STATMULTI) num_diskblocks_needed = - RoundIt((head.High - head.Low + 1) * sizeof(CAFTOCENT)) - + RoundIt(datasize - head.Free) + RoundIt(head.BlockSize); + RoundIt(toc_bytes) + + RoundIt(invalid_free_counter ? datasize + : datasize - (off_t) head.Free) + + RoundIt(head.BlockSize); if (num_diskblocks_needed > (long_int_type) fsinfo.STATAVAIL) { if (verbose) { printf("CANNOT clean %s: needs %" LLFORMAT " blocks, " @@ -1709,12 +1965,31 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) /* decide on proper size for new TOC */ newtocsize = CAF_DEFAULT_TOC_SIZE; if (head.High - newlow > newtocsize / TOC_CLEAN_RATIO) { + if ((uintmax_t) (head.High - newlow) + > ULONG_MAX / TOC_CLEAN_RATIO) { + CAFError(CAF_ERR_BADFILE); + fclose(infile); + free(tocarray); + free(newpath); + return -1; + } newtocsize = TOC_CLEAN_RATIO * (head.High - newlow); } + if (newtocsize == 0 + || (uintmax_t) newtocsize > SIZE_MAX / sizeof(CAFTOCENT)) { + CAFError(CAF_ERR_BADFILE); + fclose(infile); + free(tocarray); + free(newpath); + return -1; + } /* try to create new CAF file with some temp. pathname */ /* note: new CAF file is created in flocked state. */ - if ((fdout = CAFCreateCAFFile(path, newlow, newtocsize, statbuf.st_size, 1, + estimated_size = ((uintmax_t) statbuf.st_size > SIZE_MAX) + ? SIZE_MAX + : (size_t) statbuf.st_size; + if ((fdout = CAFCreateCAFFile(path, newlow, newtocsize, estimated_size, 1, newpath, pathlen)) < 0) { fclose(infile); @@ -1725,6 +2000,7 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) if ((outfile = fdopen(fdout, "w+")) == NULL) { CAFError(CAF_ERR_IO); + close(fdout); fclose(infile); free(tocarray); unlink(newpath); @@ -1732,7 +2008,7 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) return -1; } - newtocarray = xcalloc((head.High - newlow + 1), sizeof(CAFTOCENT)); + newtocarray = xcalloc(active_count, sizeof(CAFTOCENT)); if (fseeko(outfile, 0, SEEK_SET) < 0) { perror(newpath); @@ -1766,10 +2042,20 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) zerobuff = xcalloc(blocksize, 1); /* seek to end of output file/place to start writing new articles */ - fseeko(outfile, 0, SEEK_END); + if (fseeko(outfile, 0, SEEK_END) < 0) { + CAFError(CAF_ERR_IO); + goto errorexit; + } startoffset = ftello(outfile); + if (startoffset < 0) { + CAFError(CAF_ERR_IO); + goto errorexit; + } startoffset = CAFRoundOffsetUp(startoffset, blocksize); - fseeko(outfile, (off_t) startoffset, SEEK_SET); + if (startoffset < 0 || fseeko(outfile, startoffset, SEEK_SET) < 0) { + CAFError(CAF_ERR_IO); + goto errorexit; + } /* ** Note: startoffset will always give the start offset of the next @@ -1781,15 +2067,19 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) ** file and new TOC. */ - for (tocp = tocarray, i = head.Low; i <= head.High; ++tocp, ++i) { + for (toc_index = newlow - head.Low; toc_index < toc_count; toc_index++) { + tocp = &tocarray[toc_index]; if (tocp->Size != 0) { - newtocp = &newtocarray[i - newlow]; + newtocp = &newtocarray[toc_index - (newlow - head.Low)]; newtocp->Offset = startoffset; newtocp->Size = tocp->Size; newtocp->ModTime = tocp->ModTime; /* seek to right place in input. */ - fseeko(infile, (off_t) tocp->Offset, SEEK_SET); + if (fseeko(infile, tocp->Offset, SEEK_SET) < 0) { + CAFError(CAF_ERR_IO); + goto errorexit; + } nbytes = tocp->Size; while (nbytes > 0) { @@ -1883,8 +2173,8 @@ CAFClean(char *path, int verbose, double PercentFreeThreshold) return -1; } - if (fwrite(newtocarray, sizeof(CAFTOCENT), head.High - newlow + 1, outfile) - < head.High - newlow + 1 + if (fwrite(newtocarray, sizeof(CAFTOCENT), active_count, outfile) + < active_count || fflush(outfile) < 0) { CAFError(CAF_ERR_IO); free(newtocarray); diff --git a/storage/tradindexed/tdx-data.c b/storage/tradindexed/tdx-data.c index 2e1a584f3..d2cea8735 100644 --- a/storage/tradindexed/tdx-data.c +++ b/storage/tradindexed/tdx-data.c @@ -53,7 +53,7 @@ static int file_open(const char *base, const char *suffix, bool writable, bool append); static bool file_open_index(struct group_data *, const char *suffix); static bool file_open_data(struct group_data *, const char *suffix); -static void *map_file(int fd, size_t length, const char *base, +static void *map_file(int fd, off_t length, const char *base, const char *suffix); static bool map_index(struct group_data *data); static bool map_data(struct group_data *data); @@ -273,25 +273,29 @@ tdx_data_open_files(struct group_data *data) ** error reporting. */ static void * -map_file(int fd, size_t length, const char *base, const char *suffix) +map_file(int fd, off_t length, const char *base, const char *suffix) { char *data; + size_t size; + if (length < 0 || (uintmax_t) length > SIZE_MAX) { + errno = EOVERFLOW; + syswarn("tradindexed: %s.%s is too large", base, suffix); + return NULL; + } if (length == 0) return NULL; + size = (size_t) length; if (!innconf->tradindexedmmap) { - ssize_t status; - - data = xmalloc(length); - status = read(fd, data, length); - if ((size_t) status != length) { + data = xmalloc(size); + if (lseek(fd, 0, SEEK_SET) < 0 || xread(fd, data, length) < 0) { syswarn("tradindexed: cannot read data file %s.%s", base, suffix); free(data); return NULL; } } else { - data = mmap(NULL, length, PROT_READ, MAP_SHARED, fd, 0); + data = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0); if (data == MAP_FAILED) { syswarn("tradindexed: cannot mmap %s.%s", base, suffix); return NULL; @@ -327,7 +331,7 @@ map_index(struct group_data *data) return false; data->indexlen = st.st_size; data->index = map_file(data->indexfd, data->indexlen, data->path, "IDX"); - return (data->index == NULL && data->indexlen > 0) ? false : true; + return data->index != NULL || data->indexlen == 0; } @@ -357,7 +361,7 @@ map_data(struct group_data *data) return false; data->datalen = st.st_size; data->data = map_file(data->datafd, data->datalen, data->path, "DAT"); - return (data->data == NULL && data->datalen > 0) ? false : true; + return data->data != NULL || data->datalen == 0; } @@ -374,7 +378,7 @@ unmap_file(void *data, off_t length, const char *base, const char *suffix) if (!innconf->tradindexedmmap) free(data); else { - if (munmap(data, length) < 0) + if (munmap(data, (size_t) length) < 0) syswarn("tradindexed: cannot munmap %s.%s", base, suffix); } return; diff --git a/storage/tradindexed/tdx-group.c b/storage/tradindexed/tdx-group.c index 66638a876..3beaebab7 100644 --- a/storage/tradindexed/tdx-group.c +++ b/storage/tradindexed/tdx-group.c @@ -114,6 +114,7 @@ struct group_index { bool writable; struct group_header *header; struct group_entry *entries; + size_t mapped_size; int count; }; @@ -121,8 +122,7 @@ struct group_index { struct hashmap; /* Internal prototypes. */ -static int index_entry_count(size_t size); -static size_t index_file_size(int count); +static bool index_file_size(int count, off_t *size); static bool index_lock(int fd, enum inn_locktype type); static bool index_lock_group(int fd, ptrdiff_t offset, enum inn_locktype); static bool index_map(struct group_index *); @@ -134,21 +134,61 @@ static long index_find(struct group_index *, const char *group); /* ** Given a file size, return the number of group entries that it contains. +** Reject sizes that cannot be represented by the types used for mapping and +** indexing the file. */ -static int -index_entry_count(size_t size) +bool +tdx_index_entry_count(off_t size, int *count) { - return (size - sizeof(struct group_header)) / sizeof(struct group_entry); + uintmax_t entries; + + if (size < (off_t) sizeof(struct group_header)) { + errno = EINVAL; + return false; + } + if ((uintmax_t) size > SIZE_MAX) { + errno = EOVERFLOW; + return false; + } + entries = ((uintmax_t) size - sizeof(struct group_header)) + / sizeof(struct group_entry); + if (entries > INT_MAX) { + errno = EOVERFLOW; + return false; + } + *count = (int) entries; + return true; } /* -** Given a number of group entries, return the required file size. +** Given a number of group entries, return the required file size after +** checking that it is representable both as an off_t and as a size_t. */ -static size_t -index_file_size(int count) +static bool +index_file_size(int count, off_t *size) { - return sizeof(struct group_header) + count * sizeof(struct group_entry); + uintmax_t bytes; + + if (count < 0 + || (uintmax_t) count + > (UINTMAX_MAX - sizeof(struct group_header)) + / sizeof(struct group_entry)) { + errno = EOVERFLOW; + return false; + } + bytes = sizeof(struct group_header) + + (uintmax_t) count * sizeof(struct group_entry); + if (bytes > SIZE_MAX) { + errno = EOVERFLOW; + return false; + } + *size = (off_t) bytes; + if (*size < 0 || (uintmax_t) *size != bytes) { + errno = EOVERFLOW; + return false; + } + return true; } @@ -200,24 +240,34 @@ index_lock_group(int fd, ptrdiff_t offset, enum inn_locktype type) static bool index_map(struct group_index *index) { + off_t length; + size_t size; + if (!innconf->tradindexedmmap && index->writable) { warn("tradindexed: cannot open for writing without mmap"); return false; } + if (!index_file_size(index->count, &length)) { + syswarn("tradindexed: invalid size for %s", index->path); + return false; + } + size = (size_t) length; if (!innconf->tradindexedmmap) { - ssize_t header_size; - ssize_t entry_size; + size_t entry_size; - header_size = sizeof(struct group_header); - entry_size = index->count * sizeof(struct group_entry); - index->header = xmalloc(header_size); + entry_size = size - sizeof(struct group_header); + index->header = xmalloc(sizeof(struct group_header)); index->entries = xmalloc(entry_size); - if (read(index->fd, index->header, header_size) != header_size) { + if (lseek(index->fd, 0, SEEK_SET) < 0 + || xread(index->fd, (char *) index->header, + (off_t) sizeof(struct group_header)) + < 0) { syswarn("tradindexed: cannot read header from %s", index->path); goto fail; } - if (read(index->fd, index->entries, entry_size) != entry_size) { + if (xread(index->fd, (char *) index->entries, (off_t) entry_size) + < 0) { syswarn("tradindexed: cannot read entries from %s", index->path); goto fail; } @@ -232,12 +282,10 @@ index_map(struct group_index *index) } else { char *data; - size_t size; int flag = PROT_READ; if (index->writable) flag = PROT_READ | PROT_WRITE; - size = index_file_size(index->count); data = mmap(NULL, size, flag, MAP_SHARED, index->fd, 0); if (data == MAP_FAILED) { syswarn("tradindexed: cannot mmap %s", index->path); @@ -247,6 +295,7 @@ index_map(struct group_index *index) index->entries = (struct group_entry *) (void *) (data + sizeof(struct group_header)); + index->mapped_size = size; return true; } } @@ -258,6 +307,8 @@ file_open_group_index(struct group_index *index, struct stat *st) int open_mode; index->header = NULL; + index->entries = NULL; + index->mapped_size = 0; open_mode = index->writable ? O_RDWR | O_CREAT : O_RDONLY; index->fd = open(index->path, open_mode, ARTFILE_MODE); if (index->fd < 0) { @@ -290,33 +341,70 @@ file_open_group_index(struct group_index *index, struct stat *st) static bool index_maybe_remap(struct group_index *index, long loc) { + struct group_index replacement; struct stat st; + bool reopened = false; int count; int r; - if (loc < index->count) + if (index->header != NULL && index->entries != NULL + && loc < index->count) return true; /* Don't remap if remapping wouldn't actually help. */ r = fstat(index->fd, &st); if (r == -1) { if (errno == ESTALE) { - index_unmap(index); - if (!file_open_group_index(index, &st)) + replacement = *index; + replacement.fd = -1; + replacement.header = NULL; + replacement.entries = NULL; + replacement.mapped_size = 0; + if (!file_open_group_index(&replacement, &st)) return false; + reopened = true; } else { syswarn("tradindexed: cannot stat %s", index->path); return false; } } - count = index_entry_count(st.st_size); - if (count < loc && index->header != NULL) + if (!tdx_index_entry_count(st.st_size, &count)) { + syswarn("tradindexed: invalid size for %s", index->path); + if (reopened) + close(replacement.fd); + return false; + } + /* If the file still has exactly the number of entries currently mapped, + remapping cannot satisfy an out-of-range lookup. A different count in + either direction requires a refresh; audit deliberately passes the + on-disk count as loc to obtain a complete current mapping. */ + if (!reopened && count == index->count && index->header != NULL) return true; - /* Okay, remapping will actually help. */ + /* Build the replacement before discarding the current mapping so that a + failed mmap or read leaves the existing state usable. */ + if (!reopened) + replacement = *index; + replacement.header = NULL; + replacement.entries = NULL; + replacement.mapped_size = 0; + replacement.count = count; + if (!index_map(&replacement)) { + if (reopened) + close(replacement.fd); + return false; + } + index_unmap(index); + if (reopened) { + close(index->fd); + index->fd = replacement.fd; + } + index->header = replacement.header; + index->entries = replacement.entries; + index->mapped_size = replacement.mapped_size; index->count = count; - return index_map(index); + return true; } @@ -334,11 +422,13 @@ index_unmap(struct group_index *index) free(index->header); free(index->entries); } else { - if (munmap(index->header, index_file_size(index->count)) < 0) + if (munmap(index->header, index->mapped_size) < 0) { syswarn("tradindexed: cannot munmap %s", index->path); + } } index->header = NULL; index->entries = NULL; + index->mapped_size = 0; } @@ -349,11 +439,21 @@ index_unmap(struct group_index *index) static bool index_expand(struct group_index *index) { + struct group_index replacement; int i; + int newcount; + off_t newsize, oldsize; - index_unmap(index); - index->count += 1024; - if (ftruncate(index->fd, index_file_size(index->count)) < 0) { + if (index->count > INT_MAX - 1024 + || !index_file_size(index->count, &oldsize) + || !index_file_size(index->count + 1024, &newsize)) { + errno = EOVERFLOW; + syswarn("tradindexed: cannot expand %s", index->path); + return false; + } + + newcount = index->count + 1024; + if (ftruncate(index->fd, newsize) < 0) { syswarn("tradindexed: cannot expand %s", index->path); return false; } @@ -365,13 +465,22 @@ index_expand(struct group_index *index) state (particularly if this was the first expansion of the index file, in which case entry 0 points to entry 0 and our walking functions may go into infinite loops). Undo the file expansion. */ - if (!index_map(index)) { - index->count -= 1024; - if (ftruncate(index->fd, index_file_size(index->count)) < 0) { + replacement = *index; + replacement.header = NULL; + replacement.entries = NULL; + replacement.mapped_size = 0; + replacement.count = newcount; + if (!index_map(&replacement)) { + if (ftruncate(index->fd, oldsize) < 0) { syswarn("tradindexed: cannot shrink %s", index->path); } return false; } + index_unmap(index); + index->header = replacement.header; + index->entries = replacement.entries; + index->mapped_size = replacement.mapped_size; + index->count = newcount; /* If the magic isn't right, assume this is a new index file. */ if (index->header->magic != TDX_MAGIC) { @@ -387,7 +496,7 @@ index_expand(struct group_index *index) index->header->freelist.recno = i; } - inn_msync_page(index->header, index_file_size(index->count), MS_ASYNC); + inn_msync_page(index->header, (size_t) newsize, MS_ASYNC); return true; } @@ -409,8 +518,16 @@ tdx_index_open(bool writable) if (!file_open_group_index(index, &st)) { goto fail; } - if ((size_t) st.st_size > sizeof(struct group_header)) { - index->count = index_entry_count(st.st_size); + if (st.st_size < 0) { + errno = EOVERFLOW; + syswarn("tradindexed: invalid size for %s", index->path); + goto fail; + } + if (st.st_size > (off_t) sizeof(struct group_header)) { + if (!tdx_index_entry_count(st.st_size, &index->count)) { + syswarn("tradindexed: invalid size for %s", index->path); + goto fail; + } if (!index_map(index)) goto fail; } else { @@ -1099,10 +1216,12 @@ hashmap_load(void) free(activepath); if (active == NULL) return NULL; - if (fstat(QIOfileno(active), &st) < 0) + if (fstat(QIOfileno(active), &st) < 0 || st.st_size < 0 + || (uintmax_t) st.st_size > SIZE_MAX) { hash_size = 32 * 1024; - else - hash_size = st.st_size / 30; + } else { + hash_size = (size_t) st.st_size / 30; + } hash = hash_create(hash_size, hashmap_hash, hashmap_key, hashmap_equal, hashmap_delete); @@ -1454,10 +1573,15 @@ tdx_index_audit(bool fix) /* Make sure the size looks sensible. */ if (fstat(index->fd, &st) < 0) { syswarn("tradindexed: cannot fstat %s", index->path); + index_lock(index->fd, INN_LOCK_UNLOCK); + return; + } + if (!tdx_index_entry_count(st.st_size, &count) + || !index_file_size(count, &expected)) { + syswarn("tradindexed: invalid size for %s", index->path); + index_lock(index->fd, INN_LOCK_UNLOCK); return; } - count = index_entry_count(st.st_size); - expected = index_file_size(count); if (expected != st.st_size) { syswarn("tradindexed: %lu bytes of trailing trash in %s", (unsigned long) (st.st_size - expected), index->path); @@ -1465,7 +1589,10 @@ tdx_index_audit(bool fix) if (ftruncate(index->fd, expected) < 0) syswarn("tradindexed: cannot truncate %s", index->path); } - index_maybe_remap(index, count); + if (!index_maybe_remap(index, count)) { + index_lock(index->fd, INN_LOCK_UNLOCK); + return; + } /* Okay everything is now mapped and happy. Validate the header. */ index_audit_header(index, fix); diff --git a/storage/tradindexed/tdx-private.h b/storage/tradindexed/tdx-private.h index a8cc095f6..0d76411c1 100644 --- a/storage/tradindexed/tdx-private.h +++ b/storage/tradindexed/tdx-private.h @@ -76,6 +76,9 @@ void tdx_cache_free(struct cache *); /* tdx-group.c */ +/* Validate a group index size and return its entry count. */ +bool tdx_index_entry_count(off_t, int *); + /* Open the group index and return an opaque data structure to use for further queries. */ struct group_index *tdx_index_open(bool writable); diff --git a/storage/tradindexed/tdx-util.c b/storage/tradindexed/tdx-util.c index 5828f155c..949a44b60 100644 --- a/storage/tradindexed/tdx-util.c +++ b/storage/tradindexed/tdx-util.c @@ -338,18 +338,24 @@ group_rebuild(const char *group, const char *path) for (file = 0; file < files->count; file++) { filename = concatpath(path, files->strings[file]); article = ReadInFile(filename, &st); - size = st.st_size; if (article == NULL) { syswarn("cannot read in %s", filename); free(filename); continue; } + size = (size_t) st.st_size; /* Check to see if the article is not in wire format. If it isn't, convert it. We only check the first line ending. */ p = strchr(article, '\n'); if (p != NULL && (p == article || p[-1] != '\r')) { wireformat = wire_from_native(article, size, &length); + if (wireformat == NULL) { + syswarn("cannot convert %s to wire format", filename); + free(filename); + free(article); + continue; + } free(article); article = wireformat; size = length; diff --git a/storage/tradspool/tradspool.c b/storage/tradspool/tradspool.c index 53a7b6920..fb5f568ac 100644 --- a/storage/tradspool/tradspool.c +++ b/storage/tradspool/tradspool.c @@ -34,7 +34,7 @@ typedef struct { char *artbase; /* start of the article data -- may be mmaped */ - unsigned int artlen; /* art length. */ + size_t artlen; /* art length. */ int nextindex; char *curdirname; DIR *curdir; @@ -813,7 +813,9 @@ OpenArticle(const char *path, RETRTYPE amount) close(fd); return NULL; } - if (sb.st_size < 0 || (unsigned int) sb.st_size > UINT_MAX) { + /* Keep the historical per-article policy limit even though artlen is a + size_t. Larger corrupt spool files must not reach fatal xmalloc. */ + if (sb.st_size < 0 || (uintmax_t) sb.st_size > UINT_MAX) { SMseterror(SMERR_UNDEFINED, "article is too large"); free(art); close(fd); @@ -824,10 +826,10 @@ OpenArticle(const char *path, RETRTYPE amount) private = xmalloc(sizeof(PRIV_TRADSPOOL)); art->private = (void *) private; - private->artlen = sb.st_size; + private->artlen = (size_t) sb.st_size; if (innconf->articlemmap) { if ((private->artbase = - mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0)) + mmap(NULL, private->artlen, PROT_READ, MAP_SHARED, fd, 0)) == MAP_FAILED) { SMseterror(SMERR_UNDEFINED, NULL); syswarn("tradspool: could not mmap article %s", path); @@ -837,9 +839,9 @@ OpenArticle(const char *path, RETRTYPE amount) return NULL; } if (amount == RETR_ALL) - madvise(private->artbase, sb.st_size, MADV_WILLNEED); + madvise(private->artbase, private->artlen, MADV_WILLNEED); else - madvise(private->artbase, sb.st_size, MADV_SEQUENTIAL); + madvise(private->artbase, private->artlen, MADV_SEQUENTIAL); /* consider coexisting both wireformatted and nonwireformatted */ p = memchr(private->artbase, '\n', private->artlen); @@ -857,6 +859,15 @@ OpenArticle(const char *path, RETRTYPE amount) } else { wfarticle = wire_from_native(private->artbase, private->artlen, &wflen); + if (wfarticle == NULL) { + SMseterror(SMERR_UNDEFINED, + "article is too large after wire formatting"); + munmap(private->artbase, private->artlen); + free(art->private); + free(art); + close(fd); + return NULL; + } munmap(private->artbase, private->artlen); private->artbase = wfarticle; private->artlen = wflen; @@ -865,7 +876,7 @@ OpenArticle(const char *path, RETRTYPE amount) } else { private->mmapped = false; private->artbase = xmalloc(private->artlen); - if (xread(fd, private->artbase, private->artlen) < 0) { + if (xread(fd, private->artbase, sb.st_size) < 0) { SMseterror(SMERR_UNDEFINED, NULL); syswarn("tradspool: could not read article %s", path); free(private->artbase); @@ -888,6 +899,15 @@ OpenArticle(const char *path, RETRTYPE amount) /* need to make a wireformat copy of the article */ wfarticle = wire_from_native(private->artbase, private->artlen, &wflen); + if (wfarticle == NULL) { + SMseterror(SMERR_UNDEFINED, + "article is too large after wire formatting"); + free(private->artbase); + free(art->private); + free(art); + close(fd); + return NULL; + } free(private->artbase); private->artbase = wfarticle; private->artlen = wflen; diff --git a/support/mkmanifest b/support/mkmanifest index 70ddc7da9..e7bc56cbd 100755 --- a/support/mkmanifest +++ b/support/mkmanifest @@ -327,6 +327,7 @@ tests/lib/network/server.t tests/lib/pread.t tests/lib/pwrite.t tests/lib/qio.t +tests/lib/readin.t tests/lib/reallocarray.t tests/lib/reservedfd.t tests/lib/setenv.t @@ -345,8 +346,10 @@ tests/overview/buffindexed.t tests/overview/ovsqlite.t tests/overview/ovsqlite-read.t tests/overview/ovsqlite-write.t +tests/overview/tdx-group.t tests/overview/tradindexed.t tests/overview/xref.t tests/perl/minimum-version.t +tests/storage/caf.t tests/storage/cancel-tombstone.t tests/util/innbind.t diff --git a/tests/Makefile b/tests/Makefile index 54fdd3ecc..2a67ea5a9 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -30,12 +30,14 @@ TESTS = authprogs/ident.t expire/tombstone.t expire/tombstone-hisexpire.t \ lib/messageid.t lib/messages.t lib/mkstemp.t \ lib/network/addr-ipv4.t lib/network/addr-ipv6.t \ lib/network/client.t lib/network/server.t \ - lib/pread.t lib/pwrite.t lib/qio.t lib/reallocarray.t lib/reservedfd.t \ + lib/pread.t lib/pwrite.t lib/qio.t lib/readin.t lib/reallocarray.t \ + lib/reservedfd.t \ lib/setenv.t lib/snprintf.t lib/strlcat.t \ lib/strlcpy.t lib/tst.t lib/uwildmat.t lib/vector.t lib/wire.t \ lib/xwrite.t nnrpd/auth-ext.t overview/api.t overview/buffindexed.t \ - overview/ovsqlite.t overview/tradindexed.t overview/xref.t \ - storage/cancel-tombstone.t util/innbind.t + overview/ovsqlite.t overview/tdx-group.t overview/tradindexed.t \ + overview/xref.t \ + storage/caf.t storage/cancel-tombstone.t util/innbind.t ## Extra stuff that needs to be built before tests can be run. @@ -268,6 +270,9 @@ lib/pwrite.t: lib/pwrite.o lib/pwrite-t.o tap/basic.o $(LIBINN) lib/qio.t: lib/qio-t.o tap/basic.o $(LIBINN) $(LINK) lib/qio-t.o tap/basic.o $(LIBINN) +lib/readin.t: lib/readin-t.o tap/basic.o $(LIBINN) + $(LINK) lib/readin-t.o tap/basic.o $(LIBINN) + lib/reallocarray.o: ../lib/reallocarray.c $(CC) $(CFLAGS) -DTESTING -c -o $@ ../lib/reallocarray.c @@ -346,6 +351,9 @@ overview/ovsqlite-write.t: overview/ovsqlite-write-t.o tap/basic.o $(STORAGEDEPS $(LINKDEPS) overview/ovsqlite-write-t.o tap/basic.o $(STORAGELIBS) \ $(LIBS) +overview/tdx-group.t: overview/tdx-group-t.o tap/basic.o $(STORAGEDEPS) + $(LINKDEPS) overview/tdx-group-t.o tap/basic.o $(STORAGELIBS) $(LIBS) + overview/tradindexed-t.o: overview/overview-t.c $(CC) $(CFLAGS) -DOVTYPE=tradindexed -c -o $@ overview/overview-t.c @@ -358,6 +366,9 @@ overview/xref.t: overview/xref-t.o tap/basic.o $(STORAGEDEPS) perl/minimum-version.t: perl/minimum-version.t.in $(FIXSCRIPT) $(FIX) -i perl/minimum-version.t.in +storage/caf.t: storage/caf-t.o tap/basic.o $(STORAGEDEPS) + $(LINKDEPS) storage/caf-t.o tap/basic.o $(STORAGELIBS) $(LIBS) + storage/cancel-tombstone.t: storage/cancel-tombstone-t.o tap/basic.o \ $(STORAGEDEPS) $(LINKDEPS) storage/cancel-tombstone-t.o tap/basic.o \ diff --git a/tests/TESTS b/tests/TESTS index 5499946a1..73043fa31 100644 --- a/tests/TESTS +++ b/tests/TESTS @@ -46,6 +46,7 @@ lib/network/server lib/pread lib/pwrite lib/qio +lib/readin lib/reallocarray lib/reservedfd lib/setenv @@ -64,10 +65,12 @@ overview/buffindexed overview/overchan overview/ovsqlite overview/ovsqlite-integ +overview/tdx-group overview/tradindexed overview/xref perl/minimum-version storage/archive +storage/caf storage/cancel-tombstone storage/makehistory storage/sm diff --git a/tests/innd/artparse-t.c b/tests/innd/artparse-t.c index a6b7001f0..9029ff81f 100644 --- a/tests/innd/artparse-t.c +++ b/tests/innd/artparse-t.c @@ -120,8 +120,12 @@ ok_article(int n, const char *path, const char *error, bool slow, bool shift) enum channel_state expected; article = ReadInFile(path, &st); + if (article == NULL) + sysbail("cannot read %s", path); len = st.st_size; wire = wire_from_native(article, len, &wirelen); + if (wire == NULL) + sysbail("cannot convert %s to wire format", path); cp = fake_channel(); offset = shift ? random() % 50 : 0; cp->Start = offset; diff --git a/tests/lib/readin-t.c b/tests/lib/readin-t.c new file mode 100644 index 000000000..7a6831601 --- /dev/null +++ b/tests/lib/readin-t.c @@ -0,0 +1,76 @@ +/* Test suite for the robust descriptor reader. */ + +#define LIBTEST_NEW_FORMAT 1 + +#include "portable/system.h" + +#include +#include +#include + +#include "inn/libinn.h" +#include "tap/basic.h" + +int +main(void) +{ + static const char input[] = "test data"; + struct stat st; + char path[] = "readin-XXXXXX"; + char *contents; + char buffer[sizeof(input)]; + int fd, fds[2], oerrno; + + if (pipe(fds) < 0) + sysbail("cannot create pipe"); + plan(14); + + is_int(sizeof(input), write(fds[1], input, sizeof(input)), + "write test data"); + close(fds[1]); + is_int(0, xread(fds[0], buffer, sizeof(buffer)), "read complete input"); + ok(memcmp(buffer, input, sizeof(input)) == 0, "input matches"); + close(fds[0]); + + errno = 0; + is_int(-1, xread(-1, buffer, -1), "reject a negative length"); + oerrno = errno; + is_int(EINVAL, oerrno, "negative length sets errno"); + + if (pipe(fds) < 0) + sysbail("cannot create pipe"); + is_int(1, write(fds[1], input, 1), "write partial input"); + close(fds[1]); + errno = 0; + is_int(-1, xread(fds[0], buffer, 2), "reject premature EOF"); + oerrno = errno; + is_int(EIO, oerrno, "premature EOF sets errno"); + close(fds[0]); + + fd = mkstemp(path); + if (fd < 0) + sysbail("cannot create temporary input file"); + is_int(sizeof(input) - 1, write(fd, input, sizeof(input) - 1), + "write temporary input file"); + close(fd); + contents = ReadInFile(path, &st); + if (contents == NULL) + sysbail("cannot read temporary input file"); + is_int(sizeof(input) - 1, st.st_size, "report input file size"); + is_string(input, contents, "read and terminate input file"); + free(contents); + + fd = open(path, O_WRONLY); + if (fd < 0) + sysbail("cannot reopen temporary input file"); + errno = 0; + contents = ReadInDescriptor(fd, NULL); + oerrno = errno; + ok(contents == NULL, "descriptor read failure is reported"); + is_int(EBADF, oerrno, "descriptor read failure preserves errno"); + ok(fcntl(fd, F_GETFD) >= 0, "descriptor remains owned by caller"); + close(fd); + unlink(path); + + return 0; +} diff --git a/tests/lib/wire-t.c b/tests/lib/wire-t.c index 21542dac8..4cf7758e1 100644 --- a/tests/lib/wire-t.c +++ b/tests/lib/wire-t.c @@ -8,6 +8,7 @@ #include "portable/system.h" +#include #include #include @@ -47,10 +48,11 @@ main(void) { const char *p, *end; char *article, *wire, *native; + int oerrno; struct stat st; size_t wire_size, native_size, size; - test_init(64); + test_init(67); end = ta + sizeof(ta) - 1; p = end - 4; @@ -220,5 +222,14 @@ main(void) p = "x\r\nx"; ok(64, wire_nextline(p, p + 3) == p + 3); + /* Reject conversions whose minimum output size cannot be represented. */ + size = 1; + errno = 0; + article = wire_from_native("", SIZE_MAX, &size); + oerrno = errno; + ok(65, article == NULL); + ok_int(66, EOVERFLOW, oerrno); + ok_int(67, 0, size); + return 0; } diff --git a/tests/overview/overview-t.c b/tests/overview/overview-t.c index 2d279cd2a..dcd425dfe 100644 --- a/tests/overview/overview-t.c +++ b/tests/overview/overview-t.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "inn/hashtab.h" #include "inn/innconf.h" @@ -493,13 +494,76 @@ overview_verify_full_search(const char *data) return status; } +/* Verify that a read-only buffindexed process can remap group.index after a + writer grows it. This specifically exercises mmap protection on the rare + remap path rather than the protection used during initial open. */ +static bool +overview_verify_readonly_remap(void) +{ + char flag[] = NF_FLAG_OK_STRING; + char group[64]; + char target[] = "remap.1024"; + char byte; + int ready[2], resume[2], result[2]; + int i, low, high, count, seenflag, status; + pid_t child; + bool okay = true; + + if (pipe(ready) < 0 || pipe(resume) < 0 || pipe(result) < 0) + sysdie("Cannot create remap test pipes"); + child = fork(); + if (child < 0) + sysdie("Cannot fork remap test child"); + if (child == 0) { + close(ready[0]); + close(resume[1]); + close(result[0]); + OVclose(); + byte = OVopen(OV_READ) ? 1 : 0; + if (write(ready[1], &byte, 1) != 1 || byte == 0) + _exit(1); + if (read(resume[0], &byte, 1) != 1) + _exit(1); + byte = OVgroupstats(target, &low, &high, &count, &seenflag) ? 1 : 0; + if (write(result[1], &byte, 1) != 1) + _exit(1); + OVclose(); + _exit(0); + } + + close(ready[1]); + close(resume[0]); + close(result[1]); + if (read(ready[0], &byte, 1) != 1 || byte == 0) + okay = false; + for (i = 0; i <= 1024; i++) { + snprintf(group, sizeof(group), "remap.%d", i); + if (!OVgroupadd(group, 1, 0, flag)) + okay = false; + } + byte = 1; + if (write(resume[1], &byte, 1) != 1) + okay = false; + if (read(result[0], &byte, 1) != 1 || byte == 0) + okay = false; + close(ready[0]); + close(resume[1]); + close(result[0]); + if (waitpid(child, &status, 0) != child || !WIFEXITED(status) + || WEXITSTATUS(status) != 0) + okay = false; + return okay; +} + int main(void) { struct hash *groups; bool status; + int fd; + char trailing = 0; - test_init(21); + test_init(23); if (access("../data/overview/basic", F_OK) == 0) { if (chdir("../data") < 0) { @@ -567,9 +631,32 @@ main(void) ok(20, overview_verify_data("overview/bogus")); hash_free(groups); OVclose(); + + if (strcmp(innconf->ovmethod, "buffindexed") == 0) { + if (!overview_init()) + die("Opening the overview database failed, cannot continue"); + status = overview_verify_readonly_remap(); + OVclose(); + ok(21, status); + } else { + skip(21, "read-only remap test is buffindexed-specific"); + } + + if (strcmp(innconf->ovmethod, "buffindexed") == 0) { + fd = open("ov-tmp/group.index", O_WRONLY | O_APPEND); + if (fd < 0 || xwrite(fd, &trailing, 1) != 1) + sysbail("cannot append partial group.index entry"); + close(fd); + status = OVopen(OV_READ | OV_WRITE); + ok(22, !status); + if (status) + OVclose(); + } else { + skip(22, "partial group.index test is buffindexed-specific"); + } if (system("/bin/rm -rf ov-tmp") < 0) sysdie("Cannot rm ov-tmp"); - ok(21, true); + ok(23, true); return 0; } diff --git a/tests/overview/tdx-group-t.c b/tests/overview/tdx-group-t.c new file mode 100644 index 000000000..8e8568d3d --- /dev/null +++ b/tests/overview/tdx-group-t.c @@ -0,0 +1,39 @@ +/* Test suite for tradindexed group index size validation. */ + +#define LIBTEST_NEW_FORMAT 1 + +#include "portable/system.h" + +#include +#include + +#include "tap/basic.h" + +#include "../../storage/tradindexed/tdx-private.h" +#include "../../storage/tradindexed/tdx-structure.h" + +int +main(void) +{ + uintmax_t bytes; + off_t size; + bool valid; + int count, oerrno; + + bytes = sizeof(struct group_header) + + ((uintmax_t) INT_MAX + 1) * sizeof(struct group_entry); + size = (off_t) bytes; + if (size < 0 || (uintmax_t) size != bytes) + skip_all("off_t cannot represent the oversized test index"); + + plan(4); + ok(tdx_index_entry_count(sizeof(struct group_header), &count), + "accept an empty index"); + is_int(0, count, "empty index has no entries"); + errno = 0; + valid = tdx_index_entry_count(size, &count); + oerrno = errno; + ok(!valid, "reject an index with more than INT_MAX entries"); + is_int(EOVERFLOW, oerrno, "oversized entry count sets errno"); + return 0; +} diff --git a/tests/overview/xref-t.c b/tests/overview/xref-t.c index 8b95f1dd0..05a07ea5f 100644 --- a/tests/overview/xref-t.c +++ b/tests/overview/xref-t.c @@ -232,6 +232,8 @@ main(void) if (article == NULL) sysdie("Cannot read articles/xref"); wire = wire_from_native(article, strlen(article), &size); + if (wire == NULL) + sysdie("Cannot convert articles/xref to wire format"); free(article); handle.type = TOKEN_EMPTY; handle.data = wire; diff --git a/tests/storage/caf-t.c b/tests/storage/caf-t.c new file mode 100644 index 000000000..0fc896442 --- /dev/null +++ b/tests/storage/caf-t.c @@ -0,0 +1,238 @@ +/* Test suite for CAF file layout validation and cleaning. */ + +#define LIBTEST_NEW_FORMAT 1 + +#include "portable/system.h" + +#include +#include + +#include "inn/libinn.h" +#include "tap/basic.h" + +#define CAF_INNARDS 1 +#include "../../storage/timecaf/caf.h" + +extern int CAFClean(char *, int, double); + + +static CAFHEADER +test_header(void) +{ + CAFHEADER head; + + memset(&head, 0, sizeof(head)); + memcpy(head.Magic, CAF_MAGIC, CAF_MAGIC_LEN); + head.Low = 1; + head.High = 1; + head.NumSlots = 10; + head.BlockSize = CAF_DEFAULT_BLOCKSIZE; + head.FreeZoneIndexSize = head.BlockSize - sizeof(CAFHEADER); + head.FreeZoneTabSize = + head.FreeZoneIndexSize + + head.BlockSize * head.FreeZoneIndexSize * 8; + head.StartDataBlock = CAFRoundOffsetUp( + sizeof(CAFHEADER) + head.FreeZoneTabSize + + head.NumSlots * sizeof(CAFTOCENT), + head.BlockSize); + return head; +} + + +static void +write_test_file(const char *path, const CAFHEADER *head, + const CAFTOCENT *entry, off_t length) +{ + int fd; + + fd = open(path, O_CREAT | O_TRUNC | O_RDWR, 0600); + if (fd < 0) + sysbail("cannot create %s", path); + if (xwrite(fd, head, sizeof(*head)) != (ssize_t) sizeof(*head)) + sysbail("cannot write CAF header"); + if (entry != NULL) { + if (lseek(fd, sizeof(*head) + head->FreeZoneTabSize, SEEK_SET) < 0) + sysbail("cannot seek to CAF TOC"); + if (xwrite(fd, entry, sizeof(*entry)) != (ssize_t) sizeof(*entry)) + sysbail("cannot write CAF TOC entry"); + } + if (ftruncate(fd, length) < 0) + sysbail("cannot set CAF test file size"); + close(fd); +} + + +int +main(void) +{ + static char path[] = "caf-test"; + CAFHEADER head, read_head; + CAFBITMAP *bitmap; + CAFTOCENT entry, *toc; + ARTNUM art; + size_t length, bitmap_size; + uintmax_t bitmap_bytes; + off_t bitmap_offset, bitmap_block, bitmap_end; + unsigned int bitmap_index; + int error, fd, status; + char bit = 1; + + plan(23); + unlink(path); + memset(&entry, 0, sizeof(entry)); + + head = test_header(); + head.High = head.Low + head.NumSlots; + write_test_file(path, &head, &entry, head.StartDataBlock); + fd = CAFOpenArtRead(path, head.Low, &length); + error = caf_error; + is_int(-1, fd, "reject article lookup with High outside NumSlots"); + is_int(CAF_ERR_BADFILE, error, "invalid article range is a bad file"); + if (fd >= 0) + close(fd); + + head = test_header(); + head.StartDataBlock = sizeof(CAFHEADER); + write_test_file(path, &head, &entry, + sizeof(CAFHEADER) + sizeof(CAFTOCENT)); + fd = CAFOpenReadTOC(path, &read_head, &toc); + error = caf_error; + is_int(-1, fd, "reject a TOC overlapping the data region"); + is_int(CAF_ERR_BADFILE, error, "overlapping TOC is a bad file"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + head.FreeZoneIndexSize--; + write_test_file(path, &head, &entry, head.StartDataBlock); + art = head.Low; + status = CAFRemoveMultArts(path, 1, &art); + error = caf_error; + is_int(-1, status, "cancel rejects inconsistent bitmap metadata"); + is_int(CAF_ERR_BADFILE, error, "invalid cancel metadata is a bad file"); + + head = test_header(); + write_test_file(path, &head, NULL, sizeof(CAFHEADER)); + fd = CAFOpenReadTOC(path, &read_head, &toc); + error = caf_error; + is_int(-1, fd, "reject a TOC truncated by end of file"); + is_int(CAF_ERR_BADFILE, error, "truncated TOC is a bad file"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + write_test_file(path, &head, &entry, head.StartDataBlock); + fd = CAFOpenReadTOC(path, &read_head, &toc); + ok(fd >= 0, "accept a valid TOC layout"); + ok(fd >= 0 && toc[0].Size == 0, "read the valid TOC entry"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + head.FreeZoneIndexSize--; + write_test_file(path, &head, &entry, head.StartDataBlock); + fd = CAFOpenReadTOC(path, &read_head, &toc); + error = caf_error; + is_int(-1, fd, "reject inconsistent free bitmap metadata"); + is_int(CAF_ERR_BADFILE, error, "invalid bitmap layout is a bad file"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + head.BlockSize = 0; + write_test_file(path, &head, &entry, head.StartDataBlock); + fd = CAFOpenReadTOC(path, &read_head, &toc); + ok(fd >= 0, "accept a legacy zero block size"); + is_int(CAF_DEFAULT_BLOCKSIZE, read_head.BlockSize, + "normalize a legacy zero block size"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + entry.Offset = head.StartDataBlock + 100; + entry.Size = 10; + write_test_file(path, &head, &entry, head.StartDataBlock); + status = CAFClean(path, 0, 10.0); + is_int(0, status, "no-op cleaning tolerates a torn article entry"); + ok(access(path, F_OK) == 0, "no-op cleaning preserves the CAF file"); + + memset(&entry, 0, sizeof(entry)); + head = test_header(); + entry.Offset = head.StartDataBlock; + entry.Size = 1; + head.Free = 2; + write_test_file(path, &head, &entry, head.StartDataBlock + 1); + status = CAFClean(path, 0, 10.0); + is_int(0, status, "cleaning repairs an inflated free-space counter"); + fd = CAFOpenReadTOC(path, &read_head, &toc); + ok(fd >= 0 && read_head.Free == 0 && toc[0].Size == 1, + "repaired CAF retains its article and resets free space"); + if (fd >= 0) { + close(fd); + free(toc); + } + + head = test_header(); + entry.Offset = head.StartDataBlock; + entry.Size = 1; + head.StartDataBlock += head.BlockSize; + write_test_file(path, &head, &entry, head.StartDataBlock - head.BlockSize); + status = CAFClean(path, 0, 10.0); + is_int(-1, status, "cleaning rejects a non-empty CAF past EOF"); + ok(access(path, F_OK) == 0, "invalid non-empty CAF is preserved"); + + memset(&entry, 0, sizeof(entry)); + head = test_header(); + head.StartDataBlock += head.BlockSize; + write_test_file(path, &head, &entry, head.StartDataBlock - head.BlockSize); + status = CAFClean(path, 0, 10.0); + is_int(0, status, "cleaning removes an empty CAF with a torn boundary"); + ok(access(path, F_OK) < 0, "empty orphan CAF is unlinked"); + + /* Exercise a bitmap seek beyond UINT_MAX. The old unsigned expression + wrapped this offset back into the header before conversion to off_t. */ + bitmap_size = 65536; + bitmap_index = 65535; + bitmap_bytes = ((uintmax_t) bitmap_index + 1) * bitmap_size; + bitmap_offset = (off_t) bitmap_bytes; + bitmap_block = (off_t) ((uintmax_t) bitmap_index * bitmap_size); + bitmap_end = (off_t) (bitmap_bytes + bitmap_size); + if (bitmap_offset < 0 || bitmap_block < 0 + || bitmap_end < 0 + || (uintmax_t) bitmap_offset != bitmap_bytes + || (uintmax_t) bitmap_end != bitmap_bytes + bitmap_size) { + skip("off_t cannot represent the bitmap seek test"); + } else { + fd = open(path, O_CREAT | O_TRUNC | O_RDWR, 0600); + if (fd < 0 + || ftruncate(fd, bitmap_end) < 0 + || lseek(fd, bitmap_offset, SEEK_SET) < 0 + || xwrite(fd, &bit, 1) != 1) + sysbail("cannot create sparse bitmap seek test"); + bitmap = xcalloc(1, sizeof(*bitmap)); + bitmap->StartDataBlock = 0; + bitmap->MaxDataBlock = bitmap_block + (off_t) bitmap_size; + bitmap->BytesPerBMB = bitmap_size; + bitmap->BlockSize = bitmap_size; + bitmap->NumBMB = bitmap_index + 1; + bitmap->Blocks = xcalloc(bitmap->NumBMB, sizeof(*bitmap->Blocks)); + bitmap->Bits = xcalloc(1, 1); + is_int(1, CAFIsBlockFree(bitmap, fd, bitmap_block), + "bitmap block seek does not wrap in unsigned arithmetic"); + CAFDisposeBitmap(bitmap); + close(fd); + } + + unlink(path); + return 0; +}