From 3090be206f1a84c6ffb89f5acd2681c749a2ec64 Mon Sep 17 00:00:00 2001 From: Allan Jude Date: Thu, 16 Nov 2023 18:49:19 +0000 Subject: [PATCH] ARC changes to fix memory related crashes with encryption - Bail early in arc_buf_fill() when hdr->b_crypt_hdr.b_rabd is NULL - In arc_write(), avoid arc_hdr_free_abd() when HDR_IO_IN_PROGRESS indicates it's still in use. - Added additional zfs debug message loggging for unexpected edge cases to assist in future diagnosis. Sponsored-By: Odoo SA Sponsored-By: Klara Inc. Signed-off-by: Don Brady --- module/zfs/arc.c | 62 +++++++++++++++++++++++++++++++++++++++++++++--- module/zfs/sa.c | 4 ++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/module/zfs/arc.c b/module/zfs/arc.c index f81beab222ff..e84eae42fdea 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -2008,6 +2008,20 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, * further transforms on it. */ if (encrypted) { + if (hdr->b_crypt_hdr.b_rabd == NULL) { + zfs_dbgmsg("hdr rabd is NULL! compress=%d encrypted=%d " + "psize=%d lsize=%d objset=%llu object=%llu " + "iopro=%d hdrempty=%d hdrlock=%d refcount=%llu", + arc_hdr_get_compress(hdr), encrypted, + HDR_GET_PSIZE(hdr), HDR_GET_LSIZE(hdr), + (u_longlong_t)zb->zb_objset, + (u_longlong_t)zb->zb_object, + HDR_IO_IN_PROGRESS(hdr), HDR_EMPTY(hdr), + MUTEX_HELD(HDR_LOCK(hdr)), + (u_longlong_t)zfs_refcount_count( + &hdr->b_l1hdr.b_refcnt)); + return (ECKSUM); + } ASSERT(HDR_HAS_RABD(hdr)); abd_copy_to_buf(buf->b_data, hdr->b_crypt_hdr.b_rabd, HDR_GET_PSIZE(hdr)); @@ -2056,6 +2070,18 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, if (hash_lock != NULL) mutex_enter(hash_lock); + if (hdr->b_l1hdr.b_pabd == NULL) { + zfs_dbgmsg("in-place dnode but pabd is NULL! " + "compress=%d encrypted=%d psize=%d " + "lsize=%d objset=%llu object=%llu", + arc_hdr_get_compress(hdr), encrypted, + HDR_GET_PSIZE(hdr), HDR_GET_LSIZE(hdr), + (u_longlong_t)zb->zb_objset, + (u_longlong_t)zb->zb_object); + if (hash_lock != NULL) + mutex_exit(hash_lock); + return (EACCES); + } arc_buf_untransform_in_place(buf); if (hash_lock != NULL) mutex_exit(hash_lock); @@ -2071,6 +2097,15 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, if (ARC_BUF_SHARED(buf)) { ASSERT(arc_buf_is_shared(buf)); } else { + if (hdr->b_l1hdr.b_pabd == NULL) { + zfs_dbgmsg("hdr pabd is NULL! compress=%d " + "encrypted=%d psize=%d lsize=%d " + "objset=%llu object=%llu", + arc_hdr_get_compress(hdr), encrypted, + HDR_GET_PSIZE(hdr), HDR_GET_LSIZE(hdr), + (u_longlong_t)zb->zb_objset, + (u_longlong_t)zb->zb_object); + } abd_copy_to_buf(buf->b_data, hdr->b_l1hdr.b_pabd, arc_buf_size(buf)); } @@ -2739,6 +2774,10 @@ arc_buf_alloc_impl(arc_buf_hdr_t *hdr, spa_t *spa, const zbookmark_phys_t *zb, * hold the hash_lock or be undiscoverable. */ ASSERT(HDR_EMPTY_OR_LOCKED(hdr)); + if (!HDR_EMPTY_OR_LOCKED(hdr)) { + zfs_dbgmsg("without HDR_EMPTY_OR_LOCKED. empty=%d locked=%d", + HDR_EMPTY(hdr), MUTEX_HELD(HDR_LOCK(hdr))); + } /* * Only honor requests for compressed bufs if the hdr is actually @@ -3165,6 +3204,11 @@ arc_hdr_alloc_abd(arc_buf_hdr_t *hdr, int alloc_flags) IMPLY(alloc_rdata, HDR_PROTECTED(hdr)); if (alloc_rdata) { + if (HDR_PROTECTED(hdr) == B_FALSE) { + zfs_dbgmsg("allocating rabd on a not-encrypted HDR"); + } + ASSERT(HDR_PROTECTED(hdr)); + size = HDR_GET_PSIZE(hdr); ASSERT3P(hdr->b_crypt_hdr.b_rabd, ==, NULL); hdr->b_crypt_hdr.b_rabd = arc_get_data_abd(hdr, size, hdr, @@ -3265,8 +3309,11 @@ arc_hdr_alloc(uint64_t spa, int32_t psize, int32_t lsize, arc_hdr_set_flags(hdr, arc_bufc_to_flags(type) | ARC_FLAG_HAS_L1HDR); arc_hdr_set_compress(hdr, compression_type); hdr->b_complevel = complevel; - if (protected) + if (protected) { arc_hdr_set_flags(hdr, ARC_FLAG_PROTECTED); + if (hdr->b_crypt_hdr.b_rabd != NULL) + zfs_dbgmsg("allocating but rabd is not NULL"); + } hdr->b_l1hdr.b_state = arc_anon; hdr->b_l1hdr.b_arc_access = 0; @@ -6704,8 +6751,17 @@ arc_write(zio_t *pio, spa_t *spa, uint64_t txg, VERIFY3P(buf->b_data, !=, NULL); } - if (HDR_HAS_RABD(hdr)) - arc_hdr_free_abd(hdr, B_TRUE); + if (HDR_HAS_RABD(hdr)) { + if (HDR_IO_IN_PROGRESS(hdr)) { + zfs_dbgmsg("tried to free an rABD while IO was in " + "progress refcount=%llu", + (u_longlong_t)zfs_refcount_count( + &hdr->b_l1hdr.b_refcnt)); + } else { + arc_hdr_free_abd(hdr, B_TRUE); + } + + } if (!(zio_flags & ZIO_FLAG_RAW)) arc_hdr_set_compress(hdr, ZIO_COMPRESS_OFF); diff --git a/module/zfs/sa.c b/module/zfs/sa.c index 0ae4c331dd36..3751fef2854e 100644 --- a/module/zfs/sa.c +++ b/module/zfs/sa.c @@ -360,7 +360,7 @@ sa_attr_op(sa_handle_t *hdl, sa_bulk_attr_t *bulk, int count, } } if (error && error != ENOENT) { - return ((error == ECKSUM) ? EIO : error); + return (error); } switch (data_op) { @@ -1114,7 +1114,7 @@ sa_setup(objset_t *os, uint64_t sa_obj, const sa_attr_reg_t *reg_attrs, avl_destroy(&sa->sa_layout_num_tree); mutex_destroy(&sa->sa_lock); kmem_free(sa, sizeof (sa_os_t)); - return ((error == ECKSUM) ? EIO : error); + return (error); } void