Skip to content

Commit 0e37a0f

Browse files
sdimitrobehlendorf
authored andcommitted
Assert that a dnode's bonuslen never exceeds its recorded size
This patch introduces an assertion that can catch pitfalls in development where there is a mismatch between the size of reads and writes between a *_phys structure and its respective in-core structure when bonus buffers are used. This debugging-aid should be complementary to the verification done by ztest in ztest_verify_dnode_bt(). A side to this patch is that we now clear out any extra bytes past a bonus buffer's new size when the buffer is shrinking. Reviewed-by: Matt Ahrens <matt@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Serapheim Dimitropoulos <serapheim@delphix.com> Closes #8348
1 parent e2b31b5 commit 0e37a0f

File tree

2 files changed

+52
-0
lines changed

2 files changed

+52
-0
lines changed

module/zfs/dbuf.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3931,6 +3931,46 @@ dbuf_sync_indirect(dbuf_dirty_record_t *dr, dmu_tx_t *tx)
39313931
zio_nowait(zio);
39323932
}
39333933

3934+
#ifdef ZFS_DEBUG
3935+
/*
3936+
* Verify that the size of the data in our bonus buffer does not exceed
3937+
* its recorded size.
3938+
*
3939+
* The purpose of this verification is to catch any cases in development
3940+
* where the size of a phys structure (i.e space_map_phys_t) grows and,
3941+
* due to incorrect feature management, older pools expect to read more
3942+
* data even though they didn't actually write it to begin with.
3943+
*
3944+
* For a example, this would catch an error in the feature logic where we
3945+
* open an older pool and we expect to write the space map histogram of
3946+
* a space map with size SPACE_MAP_SIZE_V0.
3947+
*/
3948+
static void
3949+
dbuf_sync_leaf_verify_bonus_dnode(dbuf_dirty_record_t *dr)
3950+
{
3951+
dnode_t *dn = DB_DNODE(dr->dr_dbuf);
3952+
3953+
/*
3954+
* Encrypted bonus buffers can have data past their bonuslen.
3955+
* Skip the verification of these blocks.
3956+
*/
3957+
if (DMU_OT_IS_ENCRYPTED(dn->dn_bonustype))
3958+
return;
3959+
3960+
uint16_t bonuslen = dn->dn_phys->dn_bonuslen;
3961+
uint16_t maxbonuslen = DN_SLOTS_TO_BONUSLEN(dn->dn_num_slots);
3962+
ASSERT3U(bonuslen, <=, maxbonuslen);
3963+
3964+
arc_buf_t *datap = dr->dt.dl.dr_data;
3965+
char *datap_end = ((char *)datap) + bonuslen;
3966+
char *datap_max = ((char *)datap) + maxbonuslen;
3967+
3968+
/* ensure that everything is zero after our data */
3969+
for (; datap_end < datap_max; datap_end++)
3970+
ASSERT(*datap_end == 0);
3971+
}
3972+
#endif
3973+
39343974
/*
39353975
* dbuf_sync_leaf() is called recursively from dbuf_sync_list() so it is
39363976
* critical the we not allow the compiler to inline this function in to
@@ -4007,6 +4047,10 @@ dbuf_sync_leaf(dbuf_dirty_record_t *dr, dmu_tx_t *tx)
40074047
DN_MAX_BONUS_LEN(dn->dn_phys));
40084048
DB_DNODE_EXIT(db);
40094049

4050+
#ifdef ZFS_DEBUG
4051+
dbuf_sync_leaf_verify_bonus_dnode(dr);
4052+
#endif
4053+
40104054
if (*datap != db->db.db_data) {
40114055
int slots = DB_DNODE(db)->dn_num_slots;
40124056
int bonuslen = DN_SLOTS_TO_BONUSLEN(slots);

module/zfs/dnode.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,14 @@ dnode_setbonuslen(dnode_t *dn, int newsize, dmu_tx_t *tx)
390390
rw_enter(&dn->dn_struct_rwlock, RW_WRITER);
391391
ASSERT3U(newsize, <=, DN_SLOTS_TO_BONUSLEN(dn->dn_num_slots) -
392392
(dn->dn_nblkptr-1) * sizeof (blkptr_t));
393+
394+
if (newsize < dn->dn_bonuslen) {
395+
/* clear any data after the end of the new size */
396+
size_t diff = dn->dn_bonuslen - newsize;
397+
char *data_end = ((char *)dn->dn_bonus->db.db_data) + newsize;
398+
bzero(data_end, diff);
399+
}
400+
393401
dn->dn_bonuslen = newsize;
394402
if (newsize == 0)
395403
dn->dn_next_bonuslen[tx->tx_txg & TXG_MASK] = DN_ZERO_BONUSLEN;

0 commit comments

Comments
 (0)