Fix bugs involving recursive link operations that could be triggered by deleting links to their parent group

release_docs/CHANGELOG.md

@@ -494,6 +494,12 @@ Simple example programs showing how to use complex number datatypes have been ad
 # 🪲 Bug Fixes
 
 ## Library
+### 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 issue CVE-2025-6857
 

src/CMakeLists.txt

@@ -613,6 +613,7 @@ set (H5O_SOURCES
     ${HDF5_SRC_DIR}/H5Ocopy.c
     ${HDF5_SRC_DIR}/H5Ocopy_ref.c
     ${HDF5_SRC_DIR}/H5Odbg.c
+    ${HDF5_SRC_DIR}/H5Odeleted.c
     ${HDF5_SRC_DIR}/H5Odeprec.c
     ${HDF5_SRC_DIR}/H5Odrvinfo.c
     ${HDF5_SRC_DIR}/H5Odtype.c

src/H5Aint.c

@@ -74,7 +74,7 @@ typedef struct {
 
 static herr_t H5A__close_cb(H5VL_object_t *attr_vol_obj, void **request);
 static herr_t H5A__compact_build_table_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
-                                          unsigned *oh_flags_ptr, void *_udata /*in,out*/);
+                                          void *_udata /*in,out*/);
 static herr_t H5A__dense_build_table_cb(const H5A_t *attr, void *_udata);
 static int    H5A__attr_cmp_name_inc(const void *attr1, const void *attr2);
 static int    H5A__attr_cmp_name_dec(const void *attr1, const void *attr2);
@@ -1486,7 +1486,7 @@ H5A__exists_by_name(H5G_loc_t loc, const char *obj_name, const char *attr_name,
  */
 static herr_t
 H5A__compact_build_table_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
-                            unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
+                            void *_udata /*in,out*/)
 {
     H5A_compact_bt_ud_t *udata     = (H5A_compact_bt_ud_t *)_udata; /* Operator user data */
     herr_t               ret_value = H5_ITER_CONT;                  /* Return value */

src/H5Oalloc.c

@@ -938,11 +938,11 @@ H5O__alloc_chunk(H5F_t *f, H5O_t *oh, size_t size, size_t found_null, const H5O_
             for (u = 0, curr_msg = &oh->mesg[0]; u < oh->nmesgs; u++, curr_msg++)
                 if (curr_msg->chunkno == chunkno - 1) {
                     if (curr_msg->type->id == H5O_NULL_ID) {
-                        /* Delete the null message */
-                        if (u < oh->nmesgs - 1)
-                            memmove(curr_msg, curr_msg + 1, ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t));
-                        oh->nmesgs--;
-                    } /* end if */
+                        /* Delete the null message. Defer actual deletion so we don't interfere with a higher
+                         * level of recursion by moving the messages. */
+                        curr_msg->type = H5O_MSG_DELETED;
+                        oh->num_deleted_mesgs++;
+                    }
                     else {
                         assert(curr_msg->type->id != H5O_CONT_ID);
 
@@ -1037,17 +1037,10 @@ H5O__alloc_chunk(H5F_t *f, H5O_t *oh, size_t size, size_t found_null, const H5O_
                 /* Release any information/memory for message */
                 H5O__msg_free_mesg(old_null_msg);
 
-                /* Remove null message from list of messages */
-                if (found_msg->null_msgno < (oh->nmesgs - 1))
-                    memmove(old_null_msg, old_null_msg + 1,
-                            ((oh->nmesgs - 1) - found_msg->null_msgno) * sizeof(H5O_mesg_t));
-
-                /* Decrement # of messages */
-                /* (Don't bother reducing size of message array for now -QAK) */
-                oh->nmesgs--;
-
-                /* Adjust message index for new NULL message */
-                found_null--;
+                /* Delete null message from list of messages. Defer actual deletion so we don't interfere with
+                 * a higher level of recursion by moving the messages. */
+                old_null_msg->type = H5O_MSG_DELETED;
+                oh->num_deleted_mesgs++;
             } /* end if */
 
             /* Mark the new null message as dirty */
@@ -2269,6 +2262,18 @@ H5O__condense_header(H5F_t *f, H5O_t *oh)
     /* check args */
     assert(oh != NULL);
 
+    /* First remove all deleted messages from the object header */
+    for (unsigned u = 0; oh->num_deleted_mesgs > 0 && u < oh->nmesgs;)
+        if (oh->mesg[u].type->id == H5O_DELETED_ID) {
+            if (u < (oh->nmesgs - 1))
+                memmove(&oh->mesg[u], &oh->mesg[u + 1], ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t));
+            oh->nmesgs--;
+            oh->num_deleted_mesgs--;
+        }
+        else
+            u++;
+    assert(oh->num_deleted_mesgs == 0);
+
     /* Loop until no changed to the object header messages & chunks */
     do {
         /* Reset 'rescan chunks' flag */

src/H5Oattribute.c

@@ -109,24 +109,20 @@ typedef struct {
 /* Local Prototypes */
 /********************/
 static herr_t H5O__attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
-                                    unsigned *oh_modified, void *_udata);
+                                    void *_udata);
 static htri_t H5O__attr_find_opened_attr(const H5O_loc_t *loc, H5A_t **attr, const char *name_to_open);
-static herr_t H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence,
-                                unsigned H5_ATTR_UNUSED *oh_modified, void *_udata);
+static herr_t H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, void *_udata);
 static herr_t H5O__attr_open_by_idx_cb(const H5A_t *attr, void *_ret_attr);
-static herr_t H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
-                                 unsigned *oh_modified, void *_udata);
+static herr_t H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence, void *_udata);
 static herr_t H5O__attr_rename_chk_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
-                                      unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
-                                      void *_udata);
+                                      unsigned H5_ATTR_UNUSED sequence, void *_udata);
 static herr_t H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
-                                      unsigned *oh_modified, void *_udata);
+                                      void *_udata);
 static herr_t H5O__attr_remove_update(const H5O_loc_t *loc, H5O_t *oh, H5O_ainfo_t *ainfo);
 static herr_t H5O__attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
-                                  unsigned *oh_modified, void *_udata);
-static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
-                                  unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
                                   void *_udata);
+static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
+                                  unsigned H5_ATTR_UNUSED sequence, void *_udata);
 
 /*********************/
 /* Package Variables */
@@ -152,7 +148,7 @@ static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
  */
 static herr_t
 H5O__attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
-                      unsigned *oh_modified, void *_udata /*in,out*/)
+                      void *_udata /*in,out*/)
 {
     H5O_iter_cvt_t *udata     = (H5O_iter_cvt_t *)_udata; /* Operator user data */
     H5A_t          *attr      = (H5A_t *)mesg->native;    /* Pointer to attribute to insert */
@@ -178,7 +174,7 @@ H5O__attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_U
         HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, H5_ITER_ERROR, "unable to convert into null message");
 
     /* Indicate that the object header was modified */
-    *oh_modified = H5O_MODIFY_CONDENSE;
+    oh->mesgs_modified = H5O_MODIFY_CONDENSE;
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
@@ -394,8 +390,7 @@ H5O__attr_create(const H5O_loc_t *loc, H5A_t *attr)
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
-                  unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
+H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, void *_udata /*in,out*/)
 {
     H5O_iter_opn_t *udata     = (H5O_iter_opn_t *)_udata; /* Operator user data */
     herr_t          ret_value = H5_ITER_CONT;             /* Return value */
@@ -776,7 +771,7 @@ H5O__attr_update_shared(H5F_t *f, H5O_t *oh, H5A_t *attr, H5O_shared_t *update_s
  */
 static herr_t
 H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
-                   unsigned *oh_modified, void *_udata /*in,out*/)
+                   void *_udata /*in,out*/)
 {
     H5O_iter_wrt_t    *udata       = (H5O_iter_wrt_t *)_udata; /* Operator user data */
     H5O_chunk_proxy_t *chk_proxy   = NULL;                     /* Chunk that message is in */
@@ -828,7 +823,7 @@ H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUS
                             "unable to update attribute in shared storage");
 
         /* Indicate that the object header was modified */
-        *oh_modified = H5O_MODIFY;
+        oh->mesgs_modified = H5O_MODIFY;
 
         /* Indicate that the attribute was found */
         udata->found = true;
@@ -928,8 +923,7 @@ H5O__attr_write(const H5O_loc_t *loc, H5A_t *attr)
  */
 static herr_t
 H5O__attr_rename_chk_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/,
-                        unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
-                        void *_udata /*in,out*/)
+                        unsigned H5_ATTR_UNUSED sequence, void *_udata /*in,out*/)
 {
     H5O_iter_ren_t *udata     = (H5O_iter_ren_t *)_udata; /* Operator user data */
     herr_t          ret_value = H5_ITER_CONT;             /* Return value */
@@ -970,7 +964,7 @@ H5O__attr_rename_chk_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/,
  */
 static herr_t
 H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
-                        unsigned *oh_modified, void *_udata /*in,out*/)
+                        void *_udata /*in,out*/)
 {
     H5O_iter_ren_t    *udata       = (H5O_iter_ren_t *)_udata; /* Operator user data */
     H5O_chunk_proxy_t *chk_proxy   = NULL;                     /* Chunk that message is in */
@@ -1047,7 +1041,7 @@ H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR
                     HGOTO_ERROR(H5E_ATTR, H5E_CANTDELETE, H5_ITER_ERROR,
                                 "unable to release previous attribute");
 
-                *oh_modified = H5O_MODIFY_CONDENSE;
+                oh->mesgs_modified = H5O_MODIFY_CONDENSE;
 
                 /* Append renamed attribute to object header */
                 /* (Don't let it become shared) */
@@ -1065,7 +1059,7 @@ H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR
         }     /* end else */
 
         /* Indicate that the object header was modified */
-        *oh_modified |= H5O_MODIFY;
+        oh->mesgs_modified |= H5O_MODIFY;
 
         /* Indicate that we found an existing attribute with the old name */
         udata->found = true;
@@ -1415,7 +1409,7 @@ H5O__attr_remove_update(const H5O_loc_t *loc, H5O_t *oh, H5O_ainfo_t *ainfo)
  */
 static herr_t
 H5O__attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
-                    unsigned *oh_modified, void *_udata /*in,out*/)
+                    void *_udata /*in,out*/)
 {
     H5O_iter_rm_t *udata     = (H5O_iter_rm_t *)_udata; /* Operator user data */
     herr_t         ret_value = H5_ITER_CONT;            /* Return value */
@@ -1434,7 +1428,7 @@ H5O__attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNU
             HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, H5_ITER_ERROR, "unable to convert into null message");
 
         /* Indicate that the object header was modified */
-        *oh_modified = H5O_MODIFY_CONDENSE;
+        oh->mesgs_modified = H5O_MODIFY_CONDENSE;
 
         /* Indicate that this message is the attribute to be deleted */
         udata->found = true;
@@ -1674,7 +1668,7 @@ H5O__attr_count_real(H5F_t *f, H5O_t *oh, hsize_t *nattrs)
  */
 static herr_t
 H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
-                    unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
+                    void *_udata /*in,out*/)
 {
     H5O_iter_xst_t *udata     = (H5O_iter_xst_t *)_udata; /* Operator user data */
     herr_t          ret_value = H5_ITER_CONT;             /* Return value */

src/H5Odeleted.c

@@ -0,0 +1,50 @@
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Copyright by The HDF Group.                                               *
+ * All rights reserved.                                                      *
+ *                                                                           *
+ * This file is part of HDF5.  The full HDF5 copyright notice, including     *
+ * terms governing use, modification, and redistribution, is contained in    *
+ * the LICENSE file, which can be found at the root of the source code       *
+ * distribution tree, or in https://www.hdfgroup.org/licenses.               *
+ * If you do not have access to either file, you may request a copy from     *
+ * [email protected].                                                        *
+ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+/*-------------------------------------------------------------------------
+ *
+ * Created:             H5Odeleted.c
+ *
+ * Purpose:             The deleted message. This should never exist in
+ *                      the file.
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "H5Omodule.h" /* This source code file is part of the H5O module */
+
+#include "H5private.h" /* Generic Functions			*/
+#include "H5Opkg.h"    /* Object headers			*/
+
+/* This message derives from H5O message class */
+const H5O_msg_class_t H5O_MSG_DELETED[1] = {{
+    H5O_DELETED_ID, /*message id number             */
+    "deleted",      /*message name for debugging    */
+    0,              /*native message size           */
+    0,              /* messages are shareable?       */
+    NULL,           /*no decode method              */
+    NULL,           /*no encode method              */
+    NULL,           /*no copy method                */
+    NULL,           /*no size method                */
+    NULL,           /*no reset method               */
+    NULL,           /*no free method                */
+    NULL,           /*no file delete method         */
+    NULL,           /*no link method		*/
+    NULL,           /*no set share method	        */
+    NULL,           /*no can share method		*/
+    NULL,           /*no pre copy native value to file */
+    NULL,           /*no copy native value to file  */
+    NULL,           /*no post copy native value to file */
+    NULL,           /*no get creation index		*/
+    NULL,           /*no set creation index		*/
+    NULL            /*no debug method               */
+}};

src/H5Oint.c

@@ -120,7 +120,8 @@ const H5O_msg_class_t *const H5O_msg_class_g[] = {
     H5O_MSG_REFCOUNT,    /*0x0016 Object's ref. count             */
     H5O_MSG_FSINFO,      /*0x0017 Free-space manager info         */
     H5O_MSG_MDCI,        /*0x0018 Metadata cache image            */
-    H5O_MSG_UNKNOWN      /*0x0019 Placeholder for unknown message */
+    H5O_MSG_UNKNOWN,     /*0x0019 Placeholder for unknown message */
+    H5O_MSG_DELETED      /*0x001a Placeholder for deleted message */
 };
 
 /* Format version bounds for object header */
@@ -2913,6 +2914,8 @@ H5O__free(H5O_t *oh, bool H5_ATTR_NDEBUG_UNUSED force)
     /* check args */
     assert(oh);
     assert(0 == oh->rc);
+    assert(0 == oh->mesgs_modified);
+    assert(0 == oh->recursion_level);
 
     /* Destroy chunks */
     if (oh->chunk) {