Skip to content

Commit

Permalink
Improve dbuf_read() error reporting
Browse files Browse the repository at this point in the history
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 <ahamza@ixsystems.com>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
  • Loading branch information
amotin committed Mar 29, 2024
1 parent e39e20b commit 7d71dc8
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1669,10 +1669,10 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
* parent's rwlock, which would be a lock ordering violation.
*/
dmu_buf_unlock_parent(db, dblt, tag);
(void) arc_read(zio, db->db_objset->os_spa, bpp,
return (arc_read(zio, db->db_objset->os_spa, bpp,
dbuf_read_done, db, ZIO_PRIORITY_SYNC_READ, zio_flags,
&aflags, &zb);
return (err);
&aflags, &zb));

early_unlock:
DB_DNODE_EXIT(db);
mutex_exit(&db->db_mtx);
Expand Down Expand Up @@ -1759,7 +1759,7 @@ dbuf_fix_old_data(dmu_buf_impl_t *db, uint64_t txg)
}

int
dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
dbuf_read(dmu_buf_impl_t *db, zio_t *pio, uint32_t flags)
{
int err = 0;
boolean_t prefetch;
Expand Down Expand Up @@ -1822,13 +1822,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)

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

if (zio == NULL && (db->db_state == DB_NOFILL ||
if (pio == NULL && (db->db_state == DB_NOFILL ||
(db->db_blkptr != NULL && !BP_IS_HOLE(db->db_blkptr)))) {
spa_t *spa = dn->dn_objset->os_spa;
zio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
pio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
need_wait = B_TRUE;
}
err = dbuf_read_impl(db, zio, flags, dblt, FTAG);
err = dbuf_read_impl(db, pio, flags, dblt, FTAG);
/*
* dbuf_read_impl has dropped db_mtx and our parent's rwlock
* for us
Expand All @@ -1849,9 +1849,10 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
*/
if (need_wait) {
if (err == 0)
err = zio_wait(zio);
err = zio_wait(pio);
else
VERIFY0(zio_wait(zio));
(void) zio_wait(pio);
pio = NULL;
}
} else {
/*
Expand All @@ -1878,7 +1879,7 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
ASSERT(db->db_state == DB_READ ||
(flags & DB_RF_HAVESTRUCT) == 0);
DTRACE_PROBE2(blocked__read, dmu_buf_impl_t *,
db, zio_t *, zio);
db, zio_t *, pio);
cv_wait(&db->db_changed, &db->db_mtx);
}
if (db->db_state == DB_UNCACHED)
Expand All @@ -1887,6 +1888,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
}
}

if (pio && err != 0) {
zio_t *zio = zio_null(pio, pio->io_spa, NULL, NULL, NULL,
ZIO_FLAG_CANFAIL);
zio->io_error = err;
zio_nowait(zio);
}

return (err);
}

Expand Down

0 comments on commit 7d71dc8

Please sign in to comment.