Skip to content

Commit 7d71dc8

Browse files
committed
Improve dbuf_read() error reporting
Previous code reported non-ZIO errors only via return value, but not via parent ZIO. It could cause NULL-dereference panics due to dmu_buf_hold_array_by_dnode() ignoring the return value, relying solely on parent ZIO status. Reported by: Ameer Hamza <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc.
1 parent e39e20b commit 7d71dc8

File tree

1 file changed

+18
-10
lines changed

1 file changed

+18
-10
lines changed

module/zfs/dbuf.c

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,10 +1669,10 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
16691669
* parent's rwlock, which would be a lock ordering violation.
16701670
*/
16711671
dmu_buf_unlock_parent(db, dblt, tag);
1672-
(void) arc_read(zio, db->db_objset->os_spa, bpp,
1672+
return (arc_read(zio, db->db_objset->os_spa, bpp,
16731673
dbuf_read_done, db, ZIO_PRIORITY_SYNC_READ, zio_flags,
1674-
&aflags, &zb);
1675-
return (err);
1674+
&aflags, &zb));
1675+
16761676
early_unlock:
16771677
DB_DNODE_EXIT(db);
16781678
mutex_exit(&db->db_mtx);
@@ -1759,7 +1759,7 @@ dbuf_fix_old_data(dmu_buf_impl_t *db, uint64_t txg)
17591759
}
17601760

17611761
int
1762-
dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
1762+
dbuf_read(dmu_buf_impl_t *db, zio_t *pio, uint32_t flags)
17631763
{
17641764
int err = 0;
17651765
boolean_t prefetch;
@@ -1822,13 +1822,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
18221822

18231823
db_lock_type_t dblt = dmu_buf_lock_parent(db, RW_READER, FTAG);
18241824

1825-
if (zio == NULL && (db->db_state == DB_NOFILL ||
1825+
if (pio == NULL && (db->db_state == DB_NOFILL ||
18261826
(db->db_blkptr != NULL && !BP_IS_HOLE(db->db_blkptr)))) {
18271827
spa_t *spa = dn->dn_objset->os_spa;
1828-
zio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
1828+
pio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
18291829
need_wait = B_TRUE;
18301830
}
1831-
err = dbuf_read_impl(db, zio, flags, dblt, FTAG);
1831+
err = dbuf_read_impl(db, pio, flags, dblt, FTAG);
18321832
/*
18331833
* dbuf_read_impl has dropped db_mtx and our parent's rwlock
18341834
* for us
@@ -1849,9 +1849,10 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
18491849
*/
18501850
if (need_wait) {
18511851
if (err == 0)
1852-
err = zio_wait(zio);
1852+
err = zio_wait(pio);
18531853
else
1854-
VERIFY0(zio_wait(zio));
1854+
(void) zio_wait(pio);
1855+
pio = NULL;
18551856
}
18561857
} else {
18571858
/*
@@ -1878,7 +1879,7 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
18781879
ASSERT(db->db_state == DB_READ ||
18791880
(flags & DB_RF_HAVESTRUCT) == 0);
18801881
DTRACE_PROBE2(blocked__read, dmu_buf_impl_t *,
1881-
db, zio_t *, zio);
1882+
db, zio_t *, pio);
18821883
cv_wait(&db->db_changed, &db->db_mtx);
18831884
}
18841885
if (db->db_state == DB_UNCACHED)
@@ -1887,6 +1888,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
18871888
}
18881889
}
18891890

1891+
if (pio && err != 0) {
1892+
zio_t *zio = zio_null(pio, pio->io_spa, NULL, NULL, NULL,
1893+
ZIO_FLAG_CANFAIL);
1894+
zio->io_error = err;
1895+
zio_nowait(zio);
1896+
}
1897+
18901898
return (err);
18911899
}
18921900

0 commit comments

Comments
 (0)