From 6f7cb26e589cea081e71c59801eae87178967861 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 21 Dec 2011 16:24:41 +1030 Subject: [PATCH] tdb2: careful on wrap. It's much harder to wrap a 64-bit tdb2 than a 32-bit tdb1, but we should still take care against bugs. Also, we should *not* cast the length to a size_t when comparing it to the stat result, in case size_t is 32 bit. --- ccan/tdb2/check.c | 4 ++-- ccan/tdb2/free.c | 2 +- ccan/tdb2/io.c | 29 +++++++++++++++++++---------- ccan/tdb2/open.c | 2 +- ccan/tdb2/private.h | 2 +- ccan/tdb2/transaction.c | 10 +++++----- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/ccan/tdb2/check.c b/ccan/tdb2/check.c index 238a5b3a..ecd6c13c 100644 --- a/ccan/tdb2/check.c +++ b/ccan/tdb2/check.c @@ -497,8 +497,8 @@ static enum TDB_ERROR check_free(struct tdb_context *tdb, } - ecode = tdb->tdb2.io->oob(tdb, off - + frec_len(frec) + ecode = tdb->tdb2.io->oob(tdb, off, + frec_len(frec) + sizeof(struct tdb_used_record), false); if (ecode != TDB_SUCCESS) { diff --git a/ccan/tdb2/free.c b/ccan/tdb2/free.c index 1b2c552a..03ca5a4f 100644 --- a/ccan/tdb2/free.c +++ b/ccan/tdb2/free.c @@ -898,7 +898,7 @@ static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size) /* Someone else may have expanded the file, so retry. */ old_size = tdb->file->map_size; - tdb->tdb2.io->oob(tdb, tdb->file->map_size + 1, true); + tdb->tdb2.io->oob(tdb, tdb->file->map_size, 1, true); if (tdb->file->map_size != old_size) { tdb_unlock_expand(tdb, F_WRLCK); return TDB_SUCCESS; diff --git a/ccan/tdb2/io.c b/ccan/tdb2/io.c index afab0c1a..b4a6f0be 100644 --- a/ccan/tdb2/io.c +++ b/ccan/tdb2/io.c @@ -81,8 +81,8 @@ void tdb_mmap(struct tdb_context *tdb) If probe is true, len being too large isn't a failure. */ -static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len, - bool probe) +static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, + tdb_off_t off, tdb_len_t len, bool probe) { struct stat st; enum TDB_ERROR ecode; @@ -92,7 +92,16 @@ static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len, || (tdb->flags & TDB_NOLOCK) || tdb_has_expansion_lock(tdb)); - if (len <= tdb->file->map_size) + if (len + off < len) { + if (probe) + return TDB_SUCCESS; + + return tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_oob off %llu len %llu wrap\n", + (long long)off, (long long)len); + } + + if (len + off <= tdb->file->map_size) return TDB_SUCCESS; if (tdb->flags & TDB_INTERNAL) { if (probe) @@ -101,7 +110,7 @@ static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len, tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, "tdb_oob len %lld beyond internal" " malloc size %lld", - (long long)len, + (long long)(off + len), (long long)tdb->file->map_size); return TDB_ERR_IO; } @@ -120,13 +129,13 @@ static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len, tdb_unlock_expand(tdb, F_RDLCK); - if (st.st_size < (size_t)len) { + if (st.st_size < off + len) { if (probe) return TDB_SUCCESS; tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_oob len %zu beyond eof at %zu", - (size_t)len, st.st_size); + "tdb_oob len %llu beyond eof at %zu", + (long long)(off + len), st.st_size); return TDB_ERR_IO; } @@ -253,7 +262,7 @@ static enum TDB_ERROR tdb_write(struct tdb_context *tdb, tdb_off_t off, "Write to read-only database"); } - ecode = tdb->tdb2.io->oob(tdb, off + len, false); + ecode = tdb->tdb2.io->oob(tdb, off, len, false); if (ecode != TDB_SUCCESS) { return ecode; } @@ -283,7 +292,7 @@ static enum TDB_ERROR tdb_read(struct tdb_context *tdb, tdb_off_t off, { enum TDB_ERROR ecode; - ecode = tdb->tdb2.io->oob(tdb, off + len, false); + ecode = tdb->tdb2.io->oob(tdb, off, len, false); if (ecode != TDB_SUCCESS) { return ecode; } @@ -574,7 +583,7 @@ static void *tdb_direct(struct tdb_context *tdb, tdb_off_t off, size_t len, if (unlikely(!tdb->file->map_ptr)) return NULL; - ecode = tdb_oob(tdb, off + len, false); + ecode = tdb_oob(tdb, off, len, false); if (unlikely(ecode != TDB_SUCCESS)) return TDB_ERR_PTR(ecode); return (char *)tdb->file->map_ptr + off; diff --git a/ccan/tdb2/open.c b/ccan/tdb2/open.c index 02ec0eb6..e238d992 100644 --- a/ccan/tdb2/open.c +++ b/ccan/tdb2/open.c @@ -747,7 +747,7 @@ finished: if (tdb->flags & TDB_VERSION1) { ecode = tdb1_probe_length(tdb); } else { - ecode = tdb->tdb2.io->oob(tdb, tdb->file->map_size + 1, true); + ecode = tdb->tdb2.io->oob(tdb, tdb->file->map_size, 1, true); } if (unlikely(ecode != TDB_SUCCESS)) goto fail; diff --git a/ccan/tdb2/private.h b/ccan/tdb2/private.h index 2062ac29..da1f0a2c 100644 --- a/ccan/tdb2/private.h +++ b/ccan/tdb2/private.h @@ -347,7 +347,7 @@ struct tdb_methods { tdb_len_t); enum TDB_ERROR (*twrite)(struct tdb_context *, tdb_off_t, const void *, tdb_len_t); - enum TDB_ERROR (*oob)(struct tdb_context *, tdb_off_t, bool); + enum TDB_ERROR (*oob)(struct tdb_context *, tdb_off_t, tdb_len_t, bool); enum TDB_ERROR (*expand_file)(struct tdb_context *, tdb_len_t); void *(*direct)(struct tdb_context *, tdb_off_t, size_t, bool); }; diff --git a/ccan/tdb2/transaction.c b/ccan/tdb2/transaction.c index 1af1c4ac..70e664fc 100644 --- a/ccan/tdb2/transaction.c +++ b/ccan/tdb2/transaction.c @@ -345,16 +345,16 @@ static void transaction_write_existing(struct tdb_context *tdb, tdb_off_t off, /* out of bounds check during a transaction */ -static enum TDB_ERROR transaction_oob(struct tdb_context *tdb, tdb_off_t len, - bool probe) +static enum TDB_ERROR transaction_oob(struct tdb_context *tdb, + tdb_off_t off, tdb_len_t len, bool probe) { - if (len <= tdb->file->map_size || probe) { + if ((off + len >= off && off + len <= tdb->file->map_size) || probe) { return TDB_SUCCESS; } tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, "tdb_oob len %lld beyond transaction size %lld", - (long long)len, + (long long)(off + len), (long long)tdb->file->map_size); return TDB_ERR_IO; } @@ -601,7 +601,7 @@ enum TDB_ERROR tdb_transaction_start(struct tdb_context *tdb) /* make sure we know about any file expansions already done by anyone else */ - tdb->tdb2.io->oob(tdb, tdb->file->map_size + 1, true); + tdb->tdb2.io->oob(tdb, tdb->file->map_size, 1, true); tdb->tdb2.transaction->old_map_size = tdb->file->map_size; /* finally hook the io methods, replacing them with -- 2.39.2