From 18297c1923b31cfdc02e4c62cc5b2630c865cefd Mon Sep 17 00:00:00 2001 From: bmribler <39579120+bmribler@users.noreply.github.com> Date: Tue, 4 Nov 2025 13:36:59 -0500 Subject: [PATCH] Fix a crash when duplicate addesses occur (related to CVE-2025-6269) (#5924) When two entries in the cache image have the same address, the library did not fail and later crashed. The cache reconstruction functions now detect duplicate addresses. When a failure occurs during the reconstruction, the cache is not clean properly. H5C__reconstruct_cache_contents now expunges any prefetched entries that were already added to the cache during the reconstruction. --- src/H5Cimage.c | 127 +++++++++++++++++++++++++++++++++++++++++++- src/H5Cprefetched.c | 2 +- 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index 85077ec6d0c..e82d76aedd6 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -109,6 +109,28 @@ /* Local Typedefs */ /******************/ +/**************************************************************************** + * + * structure H5C_recon_entry_t + * + * This structure provides a temporary uthash table to detect duplicate + * addresses. Its fields are as follows: + * + * addr: file offset of a metadata entry. Entries are added to this + * list when they are decoded. If an entry has already existed + * in the table, error will occur. + * + * entry_ptr: pointer to the cache entry, for expunging in failure cleanup. + * + * hh: uthash hash table handle + * + ****************************************************************************/ +typedef struct H5C_recon_entry_t { + haddr_t addr; /* The file address as key */ + H5C_cache_entry_t *entry_ptr; + UT_hash_handle hh; /* Hash table handle */ +} H5C_recon_entry_t; + /********************/ /* Local Prototypes */ /********************/ @@ -116,6 +138,7 @@ /* Helper routines */ static size_t H5C__cache_image_block_entry_header_size(const H5F_t *f); static size_t H5C__cache_image_block_header_size(const H5F_t *f); +static herr_t H5C__check_for_duplicates(H5C_cache_entry_t *pf_entry_ptr, H5C_recon_entry_t **recon_table_ptr); static herr_t H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **buf, size_t buf_size); #ifndef NDEBUG /* only used in assertions */ @@ -2367,6 +2390,45 @@ done: FUNC_LEAVE_NOAPI(ret_value) } /* H5C__prep_for_file_close__scan_entries() */ +/*------------------------------------------------------------------------- + * Function: H5C__check_for_duplicates() + * + * Purpose: Detects two entries with the same address. When the + * duplicate occurs, expunge the entry from the cache. Leave the + * half-processed entry for the caller to clean up as with other failures. + * + * Return: SUCCEED on success, and FAIL on failure. + * + *------------------------------------------------------------------------- + */ +static herr_t +H5C__check_for_duplicates(H5C_cache_entry_t *pf_entry_ptr, H5C_recon_entry_t **recon_table_ptr) +{ + haddr_t addr = pf_entry_ptr->addr; + herr_t ret_value = SUCCEED; /* Return value */ + H5C_recon_entry_t *recon_entry = NULL; /* Points to an entry in the temp table */ + + FUNC_ENTER_PACKAGE + + /* Check whether the address is duplicated */ + HASH_FIND(hh, *recon_table_ptr, &addr, sizeof(haddr_t), recon_entry); + + /* Duplicate found, remove the duplicated entry */ + if (recon_entry) + HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "duplicate addresses found"); + else { + /* Insert address into the hash table for checking against later */ + if (NULL == (recon_entry = (H5C_recon_entry_t *)H5MM_malloc(sizeof(H5C_recon_entry_t)))) + HGOTO_ERROR(H5E_CACHE, H5E_CANTALLOC, FAIL, "memory allocation failed for address entry"); + recon_entry->addr = addr; + recon_entry->entry_ptr = pf_entry_ptr; + HASH_ADD(hh, *recon_table_ptr, addr, sizeof(haddr_t), recon_entry); + } + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* H5C__check_for_duplicates() */ + /*------------------------------------------------------------------------- * Function: H5C__reconstruct_cache_contents() * @@ -2383,13 +2445,17 @@ done: static herr_t H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) { - H5C_cache_entry_t *pf_entry_ptr; /* Pointer to prefetched entry */ + H5C_cache_entry_t *pf_entry_ptr = NULL; /* Pointer to prefetched entry */ H5C_cache_entry_t *parent_ptr; /* Pointer to parent of prefetched entry */ hsize_t image_len; /* Image length */ const uint8_t *p; /* Pointer into image buffer */ unsigned u, v; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ + /* Declare a uthash table to detect duplicate addresses. It will be destroyed + after decoding the cache contents */ + H5C_recon_entry_t *recon_table = NULL; /* Hash table head */ + FUNC_ENTER_PACKAGE /* Sanity checks */ @@ -2414,12 +2480,24 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) /* Reconstruct entries in image */ image_len = cache_ptr->image_len; for (u = 0; u < cache_ptr->num_entries_in_image; u++) { + /* Create the prefetched entry described by the ith * entry in cache_ptr->image_entrise. */ if (NULL == (pf_entry_ptr = H5C__reconstruct_cache_entry(f, cache_ptr, &image_len, &p))) HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "reconstruction of cache entry failed"); + /* Make sure different entries don't have the same address */ + if (H5C__check_for_duplicates(pf_entry_ptr, &recon_table) < 0) { + /* Free the half-processed entry */ + if (pf_entry_ptr->image_ptr) + H5MM_xfree(pf_entry_ptr->image_ptr); + if (pf_entry_ptr->fd_parent_count > 0 && pf_entry_ptr->fd_parent_addrs) + H5MM_xfree(pf_entry_ptr->fd_parent_addrs); + pf_entry_ptr = H5FL_FREE(H5C_cache_entry_t, pf_entry_ptr); + HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "duplicate addresses in cache"); + } + /* Note that we make no checks on available cache space before * inserting the reconstructed entry into the metadata cache. * @@ -2555,6 +2633,53 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) } /* end if */ done: + if (FAIL == ret_value) { + + /* If we failed during reconstruction, remove reconstructed entries */ + H5C_recon_entry_t *recon_entry, *tmp; + + HASH_ITER(hh, recon_table, recon_entry, tmp) + { + H5C_cache_entry_t *entry_ptr = recon_entry->entry_ptr; + haddr_t addr = entry_ptr->addr; + + /* If the entry is protected, unprotect it */ + if (entry_ptr->is_protected) + if (H5C_unprotect(f, addr, (void *)entry_ptr, H5C__DELETED_FLAG) < 0) + HDONE_ERROR(H5E_CACHE, H5E_CANTUNPROTECT, FAIL, "can't unprotect entry"); + + /* If the entry is pinned, unpin it */ + if (entry_ptr->is_pinned) + if (H5C_unpin_entry((void *)entry_ptr) < 0) + HDONE_ERROR(H5E_CACHE, H5E_CANTUNPIN, FAIL, "can't unpin entry"); + + /* Remove the unpinned and unprotected entry */ + if (H5AC_expunge_entry(f, H5AC_PREFETCHED_ENTRY, addr, H5AC__NO_FLAGS_SET) < 0) { + if (entry_ptr->image_ptr) + H5MM_xfree(entry_ptr->image_ptr); + if (entry_ptr->fd_parent_count > 0 && entry_ptr->fd_parent_addrs) + H5MM_xfree(entry_ptr->fd_parent_addrs); + entry_ptr = H5FL_FREE(H5C_cache_entry_t, entry_ptr); + HDONE_ERROR(H5E_FILE, H5E_CANTEXPUNGE, FAIL, "unable to expunge driver info block"); + } + + HASH_DEL(recon_table, recon_entry); + H5MM_xfree(recon_entry); + } + /* The temporary hash table should be empty */ + assert(recon_table == NULL); + } + /* No failure, only cleanup the temporary hash table */ + else if (recon_table) { + /* Free the temporary hash table */ + H5C_recon_entry_t *cur, *tmp; + HASH_ITER(hh, recon_table, cur, tmp) + { + HASH_DEL(recon_table, cur); + H5MM_xfree(cur); + } + } + FUNC_LEAVE_NOAPI(ret_value) } /* H5C__reconstruct_cache_contents() */ diff --git a/src/H5Cprefetched.c b/src/H5Cprefetched.c index a87b9cd2a7e..b58e21bb567 100644 --- a/src/H5Cprefetched.c +++ b/src/H5Cprefetched.c @@ -231,7 +231,7 @@ H5C__prefetched_entry_notify(H5C_notify_action_t action, void *_thing) break; case H5C_NOTIFY_ACTION_BEFORE_EVICT: - for (u = 0; u < entry_ptr->flush_dep_nparents; u++) { + for (u = entry_ptr->flush_dep_nparents - 1; u < entry_ptr->flush_dep_nparents; u--) { H5C_cache_entry_t *parent_ptr; /* Sanity checks */