summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGvozden Neskovic <[email protected]>2016-08-31 10:12:08 +0200
committerBrian Behlendorf <[email protected]>2016-09-29 15:55:41 -0700
commit031d7c2fe6afaa78943bd0a563b91fc84ace42d7 (patch)
treee30c20bc233c680f997e8b11948988c7e97f1c83
parent0b78aeae927833de580e140375a15ea5ea9d924a (diff)
fix: Shift exponent too large
Undefined operation is reported by running ztest (or zloop) compiled with GCC UndefinedBehaviorSanitizer. Error only happens on top level of dnode indirection with large enough offset values. Logically, left shift operation would work, but bit shift semantics in C, and limitation of uint64_t, do not produce desired result. Issue #5059, #4883 Signed-off-by: Gvozden Neskovic <[email protected]>
-rw-r--r--include/sys/dbuf.h3
-rw-r--r--include/sys/dnode.h5
-rw-r--r--lib/libspl/include/sys/sysmacros.h3
-rw-r--r--module/zfs/dbuf.c17
-rw-r--r--module/zfs/dnode.c19
5 files changed, 38 insertions, 9 deletions
diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h
index bf546db6f..6262f012e 100644
--- a/include/sys/dbuf.h
+++ b/include/sys/dbuf.h
@@ -270,7 +270,8 @@ typedef struct dbuf_hash_table {
kmutex_t hash_mutexes[DBUF_MUTEXES];
} dbuf_hash_table_t;
-uint64_t dbuf_whichblock(struct dnode *di, int64_t level, uint64_t offset);
+uint64_t dbuf_whichblock(const struct dnode *di, const int64_t level,
+ const uint64_t offset);
void dbuf_create_bonus(struct dnode *dn);
int dbuf_spill_set_blksz(dmu_buf_t *db, uint64_t blksz, dmu_tx_t *tx);
diff --git a/include/sys/dnode.h b/include/sys/dnode.h
index 7b4d7b9d3..fe36e5989 100644
--- a/include/sys/dnode.h
+++ b/include/sys/dnode.h
@@ -104,9 +104,8 @@ extern "C" {
#define DNODES_PER_LEVEL_SHIFT (DN_MAX_INDBLKSHIFT - SPA_BLKPTRSHIFT)
#define DNODES_PER_LEVEL (1ULL << DNODES_PER_LEVEL_SHIFT)
-/* The +2 here is a cheesy way to round up */
-#define DN_MAX_LEVELS (2 + ((DN_MAX_OFFSET_SHIFT - SPA_MINBLOCKSHIFT) / \
- (DN_MIN_INDBLKSHIFT - SPA_BLKPTRSHIFT)))
+#define DN_MAX_LEVELS (DIV_ROUND_UP(DN_MAX_OFFSET_SHIFT - SPA_MINBLOCKSHIFT, \
+ DN_MIN_INDBLKSHIFT - SPA_BLKPTRSHIFT) + 1)
#define DN_BONUS(dnp) ((void*)((dnp)->dn_bonus + \
(((dnp)->dn_nblkptr - 1) * sizeof (blkptr_t))))
diff --git a/lib/libspl/include/sys/sysmacros.h b/lib/libspl/include/sys/sysmacros.h
index c2525dd2a..f99b4d686 100644
--- a/lib/libspl/include/sys/sysmacros.h
+++ b/lib/libspl/include/sys/sysmacros.h
@@ -42,6 +42,9 @@
#ifndef ARRAY_SIZE
#define ARRAY_SIZE(a) (sizeof (a) / sizeof (a[0]))
#endif
+#ifndef DIV_ROUND_UP
+#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
+#endif
#define makedevice(maj, min) makedev(maj, min)
#define _sysconf(a) sysconf(a)
diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c
index f05190b66..e487e469f 100644
--- a/module/zfs/dbuf.c
+++ b/module/zfs/dbuf.c
@@ -918,7 +918,7 @@ dbuf_loan_arcbuf(dmu_buf_impl_t *db)
* provided.
*/
uint64_t
-dbuf_whichblock(dnode_t *dn, int64_t level, uint64_t offset)
+dbuf_whichblock(const dnode_t *dn, const int64_t level, const uint64_t offset)
{
if (dn->dn_datablkshift != 0 && dn->dn_indblkshift != 0) {
/*
@@ -940,8 +940,19 @@ dbuf_whichblock(dnode_t *dn, int64_t level, uint64_t offset)
* = offset >> (datablkshift + level *
* (indblkshift - SPA_BLKPTRSHIFT))
*/
- return (offset >> (dn->dn_datablkshift + level *
- (dn->dn_indblkshift - SPA_BLKPTRSHIFT)));
+
+ const unsigned exp = dn->dn_datablkshift +
+ level * (dn->dn_indblkshift - SPA_BLKPTRSHIFT);
+
+ if (exp >= 8 * sizeof (offset)) {
+ /* This only happens on the highest indirection level */
+ ASSERT3U(level, ==, dn->dn_nlevels - 1);
+ return (0);
+ }
+
+ ASSERT3U(exp, <, 8 * sizeof (offset));
+
+ return (offset >> exp);
} else {
ASSERT3U(offset, <, dn->dn_datablksz);
return (0);
diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c
index a54db9511..f0b03bbba 100644
--- a/module/zfs/dnode.c
+++ b/module/zfs/dnode.c
@@ -1612,6 +1612,8 @@ dnode_new_blkid(dnode_t *dn, uint64_t blkid, dmu_tx_t *tx, boolean_t have_read)
sz <= blkid && sz >= dn->dn_nblkptr; sz <<= epbs)
new_nlevels++;
+ ASSERT3U(new_nlevels, <=, DN_MAX_LEVELS);
+
if (new_nlevels > dn->dn_nlevels) {
int old_nlevels = dn->dn_nlevels;
dmu_buf_impl_t *db;
@@ -2073,7 +2075,14 @@ dnode_next_offset_level(dnode_t *dn, int flags, uint64_t *offset,
else
minfill++;
- *offset = *offset >> span;
+ if (span >= 8 * sizeof (*offset)) {
+ /* This only happens on the highest indirection level */
+ ASSERT3U((lvl - 1), ==, dn->dn_phys->dn_nlevels - 1);
+ *offset = 0;
+ } else {
+ *offset = *offset >> span;
+ }
+
for (i = BF64_GET(*offset, 0, epbs);
i >= 0 && i < epb; i += inc) {
if (BP_GET_FILL(&bp[i]) >= minfill &&
@@ -2083,7 +2092,13 @@ dnode_next_offset_level(dnode_t *dn, int flags, uint64_t *offset,
if (inc > 0 || *offset > 0)
*offset += inc;
}
- *offset = *offset << span;
+
+ if (span >= 8 * sizeof (*offset)) {
+ *offset = start;
+ } else {
+ *offset = *offset << span;
+ }
+
if (inc < 0) {
/* traversing backwards; position offset at the end */
ASSERT3U(*offset, <=, start);