Fix issue with handling of failure during discard of metadata cache entries (#5817)

When discarding a metadata cache entry after flushing it, errors
during the discard process could cause the library to skip calling
the 'free_icr' callback for the entry. This could result in resource
leaks and the inability of the cache to be fully flushed and closed
due to issues such as pinned entries remaining in the cache. This
has been fixed by noting errors during the discard process, but
attempting to fully free a cache entry before signalling that an
error has occurred.

Fixes CVE-2025-7068

Author: jhendersonHDF

release_docs/CHANGELOG.md

@@ -556,13 +556,19 @@ Added Fortran wrapper h5fdsubfiling_get_file_mapping_f() for the subfiling file
 # 🪲 Bug Fixes
 
 ## Library
+
+### Fixed security issue CVE-2025-7068
+
+   Failures during the discard process on a metadata cache entry could cause the library to skip calling the callback to free the cache entry. This could result in resource leaks and issues with flushing and closing the metadata cache during file close. This has been fixed by noting errors during the discard process, but attempting to fully free a cache entry before signalling that an error has occurred.
+
+   Fixes GitHub issue #5578
+
 ### Fix bugs in object header operations
 
    In some rare circumstances, such as deleting hard links that point to their own parent group in a file using the new file format, memory corruption could occur due to recursive operations changing data structures being operated on by multiple levels of recursion. Made changes to delay changing the data structure in a dangerous way until recursion is complete.
 
    Fixes GitHub issue #5854
 
-
 ### Fixed security issues CVE-2025-6816, CVE-2025-6856 and CVE-2025-2923
 
    A specially constructed HDF5 file could contain a corrupted object header with a continuation message that points back to itself. This could result in an internal buffer being allocated with too small of a size, leading to a heap buffer overflow. This has been fixed by checking the expected number of object header chunks against the actual value as chunks are being deserialized.
@@ -588,7 +594,7 @@ Added Fortran wrapper h5fdsubfiling_get_file_mapping_f() for the subfiling file
    Fixes GitHub issue #5861
 
 ### Fixed security issue CVE-2025-2153
-   
+
    The message flags field could be modified such that a message that is not sharable according to the share_flags field in H5O_msg_class_t can be treated as sharable. An assert has been added in H5O__msg_write_real to make sure messages that are not sharable can't be modified to shared. Additionally, the check in H5O__chunk_deserialize that catches unsharable messages being marked as sharable has been improved.
 
    Fixes GitHub issue #5329

src/H5Centry.c

@@ -63,6 +63,9 @@ static herr_t H5C__pin_entry_from_client(H5C_t *cache_ptr, H5C_cache_entry_t *en
 static herr_t H5C__unpin_entry_real(H5C_t *cache_ptr, H5C_cache_entry_t *entry_ptr, bool update_rp);
 static herr_t H5C__unpin_entry_from_client(H5C_t *cache_ptr, H5C_cache_entry_t *entry_ptr, bool update_rp);
 static herr_t H5C__generate_image(H5F_t *f, H5C_t *cache_ptr, H5C_cache_entry_t *entry_ptr);
+static herr_t H5C__discard_single_entry(H5F_t *f, H5C_t *cache_ptr, H5C_cache_entry_t *entry_ptr,
+                                        bool destroy_entry, bool free_file_space,
+                                        bool suppress_image_entry_frees);
 static herr_t H5C__verify_len_eoa(H5F_t *f, const H5C_class_t *type, haddr_t addr, size_t *len, bool actual);
 static void  *H5C__load_entry(H5F_t *f,
 #ifdef H5_HAVE_PARALLEL
@@ -753,118 +756,13 @@ H5C__flush_single_entry(H5F_t *f, H5C_cache_entry_t *entry_ptr, unsigned flags)
      * Now discard the entry if appropriate.
      */
     if (destroy) {
-        /* Sanity check */
-        assert(0 == entry_ptr->flush_dep_nparents);
-
-        /* if both suppress_image_entry_frees and entry_ptr->include_in_image
-         * are true, simply set entry_ptr->image_ptr to NULL, as we have
-         * another pointer to the buffer in an instance of H5C_image_entry_t
-         * in cache_ptr->image_entries.
-         *
-         * Otherwise, free the buffer if it exists.
-         */
-        if (suppress_image_entry_frees && entry_ptr->include_in_image)
-            entry_ptr->image_ptr = NULL;
-        else if (entry_ptr->image_ptr != NULL)
-            entry_ptr->image_ptr = H5MM_xfree(entry_ptr->image_ptr);
-
-        /* If the entry is not a prefetched entry, verify that the flush
-         * dependency parents addresses array has been transferred.
-         *
-         * If the entry is prefetched, the free_isr routine will dispose of
-         * the flush dependency parents addresses array if necessary.
-         */
-        if (!entry_ptr->prefetched) {
-            assert(0 == entry_ptr->fd_parent_count);
-            assert(NULL == entry_ptr->fd_parent_addrs);
-        } /* end if */
-
-        /* Check whether we should free the space in the file that
-         * the entry occupies
-         */
-        if (free_file_space) {
-            hsize_t fsf_size;
-
-            /* Sanity checks */
-            assert(H5_addr_defined(entry_ptr->addr));
-            assert(!H5F_IS_TMP_ADDR(f, entry_ptr->addr));
-#ifndef NDEBUG
-            {
-                size_t curr_len;
-
-                /* Get the actual image size for the thing again */
-                entry_ptr->type->image_len((void *)entry_ptr, &curr_len);
-                assert(curr_len == entry_ptr->size);
-            }
-#endif
-
-            /* If the file space free size callback is defined, use
-             * it to get the size of the block of file space to free.
-             * Otherwise use entry_ptr->size.
-             */
-            if (entry_ptr->type->fsf_size) {
-                if ((entry_ptr->type->fsf_size)((void *)entry_ptr, &fsf_size) < 0)
-                    HGOTO_ERROR(H5E_CACHE, H5E_CANTFREE, FAIL, "unable to get file space free size");
-            }    /* end if */
-            else /* no file space free size callback -- use entry size */
-                fsf_size = entry_ptr->size;
-
-            /* Release the space on disk */
-            if (H5MF_xfree(f, entry_ptr->type->mem_type, entry_ptr->addr, fsf_size) < 0)
-                HGOTO_ERROR(H5E_CACHE, H5E_CANTFREE, FAIL, "unable to free file space for cache entry");
-        } /* end if ( free_file_space ) */
-
-        /* Reset the pointer to the cache the entry is within. -QAK */
-        entry_ptr->cache_ptr = NULL;
-
-        /* increment entries_removed_counter and set
-         * last_entry_removed_ptr.  As we are likely abuut to
-         * free the entry, recall that last_entry_removed_ptr
-         * must NEVER be dereferenced.
-         *
-         * Recall that these fields are maintained to allow functions
-         * that perform scans of lists of entries to detect the
-         * unexpected removal of entries (via expunge, eviction,
-         * or take ownership at present), so that they can re-start
-         * their scans if necessary.
-         *
-         * Also check if the entry we are watching for removal is being
-         * removed (usually the 'next' entry for an iteration) and reset
-         * it to indicate that it was removed.
-         */
-        cache_ptr->entries_removed_counter++;
-        cache_ptr->last_entry_removed_ptr = entry_ptr;
-
-        if (entry_ptr == cache_ptr->entry_watched_for_removal)
-            cache_ptr->entry_watched_for_removal = NULL;
-
-        /* Check for actually destroying the entry in memory */
-        /* (As opposed to taking ownership of it) */
-        if (destroy_entry) {
-            if (entry_ptr->is_dirty) {
-                /* Reset dirty flag */
-                entry_ptr->is_dirty = false;
-
-                /* If the entry's type has a 'notify' callback send a
-                 * 'entry cleaned' notice now that the entry is fully
-                 * integrated into the cache.
-                 */
-                if (entry_ptr->type->notify &&
-                    (entry_ptr->type->notify)(H5C_NOTIFY_ACTION_ENTRY_CLEANED, entry_ptr) < 0)
-                    HGOTO_ERROR(H5E_CACHE, H5E_CANTNOTIFY, FAIL,
-                                "can't notify client about entry dirty flag cleared");
-            } /* end if */
-
-            /* verify that the image has been freed */
-            assert(entry_ptr->image_ptr == NULL);
+        /* Make sure one of either `destroy_entry` or `take_ownership` are true */
+        assert(destroy_entry != take_ownership);
 
-            if (entry_ptr->type->free_icr((void *)entry_ptr) < 0)
-                HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "free_icr callback failed");
-        } /* end if */
-        else {
-            assert(take_ownership);
-        } /* end else */
-    }     /* if (destroy) */
+        if (H5C__discard_single_entry(f, cache_ptr, entry_ptr, destroy_entry, free_file_space,
+                                      suppress_image_entry_frees) < 0)
+            HGOTO_ERROR(H5E_CACHE, H5E_CANTFREE, FAIL, "can't discard cache entry");
+    }
 
     /* Check if we have to update the page buffer with cleared entries
      * so it doesn't go out of date
@@ -891,6 +789,145 @@ H5C__flush_single_entry(H5F_t *f, H5C_cache_entry_t *entry_ptr, unsigned flags)
     FUNC_LEAVE_NOAPI(ret_value)
 } /* H5C__flush_single_entry() */
 
+/*-------------------------------------------------------------------------
+ * Function:    H5C__discard_single_entry
+ *
+ * Purpose:     Helper routine to discard a cache entry, freeing as much
+ *              of the relevant file space and data structures as possible
+ *              along the way.
+ *
+ * Return:      FAIL if error is detected, SUCCEED otherwise.
+ *
+ *-------------------------------------------------------------------------
+ */
+static herr_t
+H5C__discard_single_entry(H5F_t *f, H5C_t *cache_ptr, H5C_cache_entry_t *entry_ptr, bool destroy_entry,
+                          bool free_file_space, bool suppress_image_entry_frees)
+{
+    herr_t ret_value = SUCCEED;
+
+    FUNC_ENTER_PACKAGE
+
+    assert(f);
+    assert(cache_ptr);
+    assert(entry_ptr);
+
+    /* Sanity check */
+    assert(0 == entry_ptr->flush_dep_nparents);
+
+    /* if both suppress_image_entry_frees and entry_ptr->include_in_image
+     * are true, simply set entry_ptr->image_ptr to NULL, as we have
+     * another pointer to the buffer in an instance of H5C_image_entry_t
+     * in cache_ptr->image_entries.
+     *
+     * Otherwise, free the buffer if it exists.
+     */
+    if (suppress_image_entry_frees && entry_ptr->include_in_image)
+        entry_ptr->image_ptr = NULL;
+    else if (entry_ptr->image_ptr != NULL)
+        entry_ptr->image_ptr = H5MM_xfree(entry_ptr->image_ptr);
+
+    /* If the entry is not a prefetched entry, verify that the flush
+     * dependency parents addresses array has been transferred.
+     *
+     * If the entry is prefetched, the free_icr routine will dispose of
+     * the flush dependency parents addresses array if necessary.
+     */
+    if (!entry_ptr->prefetched) {
+        assert(0 == entry_ptr->fd_parent_count);
+        assert(NULL == entry_ptr->fd_parent_addrs);
+    } /* end if */
+
+    /* Check whether we should free the space in the file that
+     * the entry occupies
+     */
+    if (free_file_space) {
+        hsize_t fsf_size;
+
+        /* Sanity checks */
+        assert(H5_addr_defined(entry_ptr->addr));
+        assert(!H5F_IS_TMP_ADDR(f, entry_ptr->addr));
+#ifndef NDEBUG
+        {
+            size_t curr_len;
+
+            /* Get the actual image size for the thing again */
+            entry_ptr->type->image_len((void *)entry_ptr, &curr_len);
+            assert(curr_len == entry_ptr->size);
+        }
+#endif
+
+        /* If the file space free size callback is defined, use
+         * it to get the size of the block of file space to free.
+         * Otherwise use entry_ptr->size.
+         */
+        if (entry_ptr->type->fsf_size) {
+            if ((entry_ptr->type->fsf_size)((void *)entry_ptr, &fsf_size) < 0)
+                /* Note error but keep going */
+                HDONE_ERROR(H5E_CACHE, H5E_CANTFREE, FAIL, "unable to get file space free size");
+        }    /* end if */
+        else /* no file space free size callback -- use entry size */
+            fsf_size = entry_ptr->size;
+
+        /* Release the space on disk */
+        if ((ret_value >= 0) && H5MF_xfree(f, entry_ptr->type->mem_type, entry_ptr->addr, fsf_size) < 0)
+            /* Note error but keep going */
+            HDONE_ERROR(H5E_CACHE, H5E_CANTFREE, FAIL, "unable to free file space for cache entry");
+    } /* end if ( free_file_space ) */
+
+    /* Reset the pointer to the cache the entry is within. -QAK */
+    entry_ptr->cache_ptr = NULL;
+
+    /* increment entries_removed_counter and set
+     * last_entry_removed_ptr.  As we are likely about to
+     * free the entry, recall that last_entry_removed_ptr
+     * must NEVER be dereferenced.
+     *
+     * Recall that these fields are maintained to allow functions
+     * that perform scans of lists of entries to detect the
+     * unexpected removal of entries (via expunge, eviction,
+     * or take ownership at present), so that they can re-start
+     * their scans if necessary.
+     *
+     * Also check if the entry we are watching for removal is being
+     * removed (usually the 'next' entry for an iteration) and reset
+     * it to indicate that it was removed.
+     */
+    cache_ptr->entries_removed_counter++;
+    cache_ptr->last_entry_removed_ptr = entry_ptr;
+
+    if (entry_ptr == cache_ptr->entry_watched_for_removal)
+        cache_ptr->entry_watched_for_removal = NULL;
+
+    /* Check for actually destroying the entry in memory */
+    /* (As opposed to taking ownership of it) */
+    if (destroy_entry) {
+        if (entry_ptr->is_dirty) {
+            /* Reset dirty flag */
+            entry_ptr->is_dirty = false;
+
+            /* If the entry's type has a 'notify' callback send a
+             * 'entry cleaned' notice now that the entry is fully
+             * integrated into the cache.
+             */
+            if (entry_ptr->type->notify &&
+                (entry_ptr->type->notify)(H5C_NOTIFY_ACTION_ENTRY_CLEANED, entry_ptr) < 0)
+                /* Note error but keep going */
+                HDONE_ERROR(H5E_CACHE, H5E_CANTNOTIFY, FAIL,
+                            "can't notify client about entry dirty flag cleared");
+        } /* end if */
+
+        /* verify that the image has been freed */
+        assert(entry_ptr->image_ptr == NULL);
+
+        if (entry_ptr->type->free_icr((void *)entry_ptr) < 0)
+            /* Note error but keep going */
+            HDONE_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "free_icr callback failed");
+    } /* end if */
+
+    FUNC_LEAVE_NOAPI(ret_value)
+} /* end H5C__discard_single_entry() */
+
 /*-------------------------------------------------------------------------
  * Function:    H5C__verify_len_eoa
  *