diff options
| author | Gvozden Neskovic <[email protected]> | 2016-08-31 10:12:08 +0200 |
|---|---|---|
| committer | Brian Behlendorf <[email protected]> | 2016-09-29 15:55:41 -0700 |
| commit | 031d7c2fe6afaa78943bd0a563b91fc84ace42d7 (patch) | |
| tree | e30c20bc233c680f997e8b11948988c7e97f1c83 /module/zfs | |
| parent | 0b78aeae927833de580e140375a15ea5ea9d924a (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]>
Diffstat (limited to 'module/zfs')
| -rw-r--r-- | module/zfs/dbuf.c | 17 | ||||
| -rw-r--r-- | module/zfs/dnode.c | 19 |
2 files changed, 31 insertions, 5 deletions
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); |