From 5f6d0b6f5aa9af2ee5be74ac415a574b732c2c0f Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Wed, 10 Sep 2014 11:59:03 -0700 Subject: [PATCH] Handle block pointers with a corrupt logical size The general strategy used by ZFS to verify that blocks are valid is to checksum everything. This has the advantage of being extremely robust and generically applicable regardless of the contents of the block. If a blocks checksum is valid then its contents are trusted by the higher layers. This system works exceptionally well as long as bad data is never written with a valid checksum. If this does somehow occur due to a software bug or a memory bit-flip on a non-ECC system it may result in kernel panic. One such place where this could occur is if somehow the logical size stored in a block pointer exceeds the maximum block size. This will result in an attempt to allocate a buffer greater than the maximum block size causing a system panic. To prevent this from happening the arc_read() function has been updated to detect this specific case. If a block pointer with an invalid logical size is passed it will treat the block as if it contained a checksum error. Signed-off-by: Tim Chase Signed-off-by: Brian Behlendorf Closes #2678 --- include/sys/arc.h | 6 +++--- module/zfs/arc.c | 24 ++++++++++++++++++++---- module/zfs/dbuf.c | 18 +++++++++++------- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/include/sys/arc.h b/include/sys/arc.h index 6328392bee23..215c75b6dfa3 100644 --- a/include/sys/arc.h +++ b/include/sys/arc.h @@ -125,15 +125,15 @@ typedef struct arc_buf_info { void arc_space_consume(uint64_t space, arc_space_type_t type); void arc_space_return(uint64_t space, arc_space_type_t type); -arc_buf_t *arc_buf_alloc(spa_t *spa, int size, void *tag, +arc_buf_t *arc_buf_alloc(spa_t *spa, uint64_t size, void *tag, arc_buf_contents_t type); -arc_buf_t *arc_loan_buf(spa_t *spa, int size); +arc_buf_t *arc_loan_buf(spa_t *spa, uint64_t size); void arc_return_buf(arc_buf_t *buf, void *tag); void arc_loan_inuse_buf(arc_buf_t *buf, void *tag); void arc_buf_add_ref(arc_buf_t *buf, void *tag); boolean_t arc_buf_remove_ref(arc_buf_t *buf, void *tag); void arc_buf_info(arc_buf_t *buf, arc_buf_info_t *abi, int state_index); -int arc_buf_size(arc_buf_t *buf); +uint64_t arc_buf_size(arc_buf_t *buf); void arc_release(arc_buf_t *buf, void *tag); int arc_released(arc_buf_t *buf); void arc_buf_sigsegv(int sig, siginfo_t *si, void *unused); diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 2c7abe6ec542..b42bb4050032 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -1434,12 +1434,12 @@ arc_space_return(uint64_t space, arc_space_type_t type) } arc_buf_t * -arc_buf_alloc(spa_t *spa, int size, void *tag, arc_buf_contents_t type) +arc_buf_alloc(spa_t *spa, uint64_t size, void *tag, arc_buf_contents_t type) { arc_buf_hdr_t *hdr; arc_buf_t *buf; - ASSERT3U(size, >, 0); + VERIFY3U(size, <=, SPA_MAXBLOCKSIZE); hdr = kmem_cache_alloc(hdr_cache, KM_PUSHPAGE); ASSERT(BUF_EMPTY(hdr)); hdr->b_size = size; @@ -1477,7 +1477,7 @@ static char *arc_onloan_tag = "onloan"; * freed. */ arc_buf_t * -arc_loan_buf(spa_t *spa, int size) +arc_loan_buf(spa_t *spa, uint64_t size) { arc_buf_t *buf; @@ -1837,7 +1837,7 @@ arc_buf_remove_ref(arc_buf_t *buf, void* tag) return (no_callback); } -int +uint64_t arc_buf_size(arc_buf_t *buf) { return (buf->b_hdr->b_size); @@ -3307,6 +3307,22 @@ arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp, arc_done_func_t *done, enum zio_compress b_compress = ZIO_COMPRESS_OFF; uint64_t b_asize = 0; + /* + * Gracefully handle a damaged logical block size as a + * checksum error by passing a dummy zio to the done callback. + */ + if (size > SPA_MAXBLOCKSIZE) { + if (done) { + rzio = zio_null(pio, spa, NULL, + NULL, NULL, zio_flags); + rzio->io_error = ECKSUM; + done(rzio, buf, private); + zio_nowait(rzio); + } + rc = ECKSUM; + goto out; + } + if (hdr == NULL) { /* this block is not in the cache */ arc_buf_hdr_t *exists = NULL; diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 76a8a99ab883..b31957094e08 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -571,12 +571,13 @@ dbuf_read_done(zio_t *zio, arc_buf_t *buf, void *vdb) dbuf_rele_and_unlock(db, NULL); } -static void +static int dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t *flags) { dnode_t *dn; zbookmark_phys_t zb; uint32_t aflags = ARC_NOWAIT; + int err; DB_DNODE_ENTER(db); dn = DB_DNODE(db); @@ -601,7 +602,7 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t *flags) dbuf_update_data(db); db->db_state = DB_CACHED; mutex_exit(&db->db_mtx); - return; + return (0); } /* @@ -621,7 +622,7 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t *flags) db->db_state = DB_CACHED; *flags |= DB_RF_CACHED; mutex_exit(&db->db_mtx); - return; + return (0); } DB_DNODE_EXIT(db); @@ -640,12 +641,14 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t *flags) dbuf_add_ref(db, NULL); - (void) arc_read(zio, db->db_objset->os_spa, db->db_blkptr, + err = arc_read(zio, db->db_objset->os_spa, db->db_blkptr, dbuf_read_done, db, ZIO_PRIORITY_SYNC_READ, (*flags & DB_RF_CANFAIL) ? ZIO_FLAG_CANFAIL : ZIO_FLAG_MUSTSUCCEED, &aflags, &zb); if (aflags & ARC_CACHED) *flags |= DB_RF_CACHED; + + return (SET_ERROR(err)); } int @@ -688,11 +691,12 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags) if (zio == NULL) zio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL); - dbuf_read_impl(db, zio, &flags); + + err = dbuf_read_impl(db, zio, &flags); /* dbuf_read_impl has dropped db_mtx for us */ - if (prefetch) + if (!err && prefetch) dmu_zfetch(&dn->dn_zfetch, db->db.db_offset, db->db.db_size, flags & DB_RF_CACHED); @@ -700,7 +704,7 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags) rw_exit(&dn->dn_struct_rwlock); DB_DNODE_EXIT(db); - if (!havepzio) + if (!err && !havepzio) err = zio_wait(zio); } else { /*