c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-12-25 18:59:33 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix theoretical "use-after-free" in dbuf_prefetch_indirect_done()

Browse Source 
Coverity complains about a "use-after-free" bug in
`dbuf_prefetch_indirect_done()` because we use a pointer value after
freeing its buffer. The pointer is used for refcounting in ARC (as the
reference holder). There is a theoretical situation where the pointer
would be reused in a way that causes the refcounting to collide, so we
change the order in which we call arc_buf_destroy() and
dbuf_prefetch_fini() to match the rest of the function. This prevents
the theoretical situation from being a possibility.

Also, we have a few return statements with a value, despite this being a
void function. We clean those up while we are making changes here.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Neal Gompa <ngompa@datto.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13869
This commit is contained in:
Richard Yao 2022-09-13 20:58:29 -04:00 committed by GitHub
parent fcd7293d4e
commit d954ca19ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 6 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
module/zfs/dbuf.c
View File

@ -3254,7 +3254,8 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb,
if (abuf == NULL) { if (abuf == NULL) {
ASSERT(zio == NULL || zio->io_error != 0); ASSERT(zio == NULL || zio->io_error != 0);
return (dbuf_prefetch_fini(dpa, B_TRUE)); dbuf_prefetch_fini(dpa, B_TRUE);
return;
} }
ASSERT(zio == NULL || zio->io_error == 0); ASSERT(zio == NULL || zio->io_error == 0);
@ -3287,7 +3288,8 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb,
dpa->dpa_curlevel, curblkid, FTAG); dpa->dpa_curlevel, curblkid, FTAG);
if (db == NULL) { if (db == NULL) {
arc_buf_destroy(abuf, private); arc_buf_destroy(abuf, private);
return (dbuf_prefetch_fini(dpa, B_TRUE)); dbuf_prefetch_fini(dpa, B_TRUE);
return;
} }
(void) dbuf_read(db, NULL, (void) dbuf_read(db, NULL,
DB_RF_MUST_SUCCEED | DB_RF_NOPREFETCH | DB_RF_HAVESTRUCT); DB_RF_MUST_SUCCEED | DB_RF_NOPREFETCH | DB_RF_HAVESTRUCT);
@ -3305,7 +3307,9 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb,
dpa->dpa_dnode->dn_objset->os_dsl_dataset, dpa->dpa_dnode->dn_objset->os_dsl_dataset,
SPA_FEATURE_REDACTED_DATASETS)); SPA_FEATURE_REDACTED_DATASETS));
if (BP_IS_HOLE(bp) || BP_IS_REDACTED(bp)) { if (BP_IS_HOLE(bp) || BP_IS_REDACTED(bp)) {
arc_buf_destroy(abuf, private);
dbuf_prefetch_fini(dpa, B_TRUE); dbuf_prefetch_fini(dpa, B_TRUE);
return;
} else if (dpa->dpa_curlevel == dpa->dpa_zb.zb_level) { } else if (dpa->dpa_curlevel == dpa->dpa_zb.zb_level) {
ASSERT3U(nextblkid, ==, dpa->dpa_zb.zb_blkid); ASSERT3U(nextblkid, ==, dpa->dpa_zb.zb_blkid);
dbuf_issue_final_prefetch(dpa, bp); dbuf_issue_final_prefetch(dpa, bp);