diff --git a/release_docs/CHANGELOG.md b/release_docs/CHANGELOG.md index 1a9617a1115..35d021e98c1 100644 --- a/release_docs/CHANGELOG.md +++ b/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 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cf71890c450..61cba80bb73 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/H5Aint.c b/src/H5Aint.c index ae19ceb2788..b65d014add2 100644 --- a/src/H5Aint.c +++ b/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 */ diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c index 14421c03635..ca7667cacc6 100644 --- a/src/H5Oalloc.c +++ b/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 */ diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c index 39637e48669..e82bff57198 100644 --- a/src/H5Oattribute.c +++ b/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 */ diff --git a/src/H5Odeleted.c b/src/H5Odeleted.c new file mode 100644 index 00000000000..92025a09501 --- /dev/null +++ b/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 * + * help@hdfgroup.org. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + +/*------------------------------------------------------------------------- + * + * 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 */ +}}; diff --git a/src/H5Oint.c b/src/H5Oint.c index bf62ad35a9e..2fa747acbc4 100644 --- a/src/H5Oint.c +++ b/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) { diff --git a/src/H5Omessage.c b/src/H5Omessage.c index 9b536c56116..12daabfbe85 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -64,7 +64,7 @@ typedef struct { static herr_t H5O__msg_reset_real(const H5O_msg_class_t *type, void *native); static herr_t H5O__msg_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, - unsigned *oh_modified, void *_udata /*in,out*/); + void *_udata /*in,out*/); static herr_t H5O__copy_mesg(H5F_t *f, H5O_t *oh, size_t idx, const H5O_msg_class_t *type, const void *mesg, unsigned mesg_flags, unsigned update_flags); @@ -944,8 +944,7 @@ H5O_msg_remove_op(const H5O_loc_t *loc, unsigned type_id, int sequence, H5O_oper *------------------------------------------------------------------------- */ static herr_t -H5O__msg_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, unsigned *oh_modified, - void *_udata /*in,out*/) +H5O__msg_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, void *_udata /*in,out*/) { H5O_iter_rm_t *udata = (H5O_iter_rm_t *)_udata; /* Operator user data */ htri_t try_remove = false; /* Whether to try removing a message */ @@ -981,7 +980,7 @@ H5O__msg_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, un HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, H5_ITER_ERROR, "unable to release message"); /* Indicate that the object header was modified */ - *oh_modified = H5O_MODIFY_CONDENSE; + oh->mesgs_modified = H5O_MODIFY_CONDENSE; /* Break out now, if we've found the correct message */ if (udata->sequence == H5O_FIRST || udata->sequence != H5O_ALL) @@ -1137,11 +1136,10 @@ herr_t H5O__msg_iterate_real(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, const H5O_mesg_operator_t *op, void *op_data) { - H5O_mesg_t *idx_msg; /* Pointer to current message */ - unsigned idx; /* Absolute index of current message in all messages */ - unsigned sequence; /* Relative index of current message for messages of type */ - unsigned oh_modified = 0; /* Whether the callback modified the object header */ - herr_t ret_value = H5_ITER_CONT; /* Return value */ + H5O_mesg_t *idx_msg; /* Pointer to current message */ + unsigned idx; /* Absolute index of current message in all messages */ + unsigned sequence; /* Relative index of current message for messages of type */ + herr_t ret_value = H5_ITER_CONT; /* Return value */ FUNC_ENTER_PACKAGE @@ -1152,6 +1150,9 @@ H5O__msg_iterate_real(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, const H5 assert(op); assert(op->u.app_op); + /* Advance recursion level */ + oh->recursion_level++; + /* Iterate over messages */ for (sequence = 0, idx = 0, idx_msg = &oh->mesg[0]; idx < oh->nmesgs && !ret_value; idx++, idx_msg++) { if (type == idx_msg->type) { @@ -1160,7 +1161,7 @@ H5O__msg_iterate_real(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, const H5 /* Check for making an "internal" (i.e. within the H5O package) callback */ if (op->op_type == H5O_MESG_OP_LIB) - ret_value = (op->u.lib_op)(oh, idx_msg, sequence, &oh_modified, op_data); + ret_value = (op->u.lib_op)(oh, idx_msg, sequence, op_data); else ret_value = (op->u.app_op)(idx_msg->native, sequence, op_data); @@ -1178,14 +1179,18 @@ H5O__msg_iterate_real(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, const H5 HERROR(H5E_OHDR, H5E_CANTLIST, "iterator function failed"); done: - /* Check if object message was modified */ - if (oh_modified) { + /* Reduce recursion level */ + oh->recursion_level--; + + /* If we are at the root of recursion (we are not inside any other msg_iterate), check if any message was + * modified (handle deferred operations) */ + if (oh->recursion_level == 0 && oh->mesgs_modified) { /* Try to condense object header info */ /* (Since this routine is used to remove messages from an * object header, the header will be condensed after each * message removal) */ - if (oh_modified & H5O_MODIFY_CONDENSE) + if (oh->mesgs_modified & H5O_MODIFY_CONDENSE) if (H5O__condense_header(f, oh) < 0) HDONE_ERROR(H5E_OHDR, H5E_CANTPACK, FAIL, "can't pack object header"); @@ -1195,7 +1200,10 @@ H5O__msg_iterate_real(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, const H5 /* Mark object header as dirty in cache */ if (H5AC_mark_entry_dirty(oh) < 0) - HGOTO_ERROR(H5E_OHDR, H5E_CANTMARKDIRTY, FAIL, "unable to mark object header as dirty"); + HDONE_ERROR(H5E_OHDR, H5E_CANTMARKDIRTY, FAIL, "unable to mark object header as dirty"); + + /* Reset mesgs_modified */ + oh->mesgs_modified = 0; } /* end if */ FUNC_LEAVE_NOAPI(ret_value) @@ -1740,6 +1748,29 @@ H5O__msg_alloc(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, unsigned *mesg_ *mesg_idx = new_idx; done: + /* If we are not in a recursive call, remove any deleted messages from the object header */ + if (oh->recursion_level == 0) { + for (unsigned u = 0; oh->num_deleted_mesgs > 0 && u < oh->nmesgs;) + if (oh->mesg[u].type->id == H5O_DELETED_ID) { + /* Check if mesg_idx is in the section to be moved, and adjust it if so */ + assert(u != *mesg_idx); + if (*mesg_idx > u) + (*mesg_idx)--; + + /* Slide down mesg array and adjust message counts */ + 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); + } + else + /* Otherwise, mark that we should condense the header when we leave recursion */ + oh->mesgs_modified = H5O_MODIFY_CONDENSE; + FUNC_LEAVE_NOAPI(ret_value) } /* end H5O__msg_alloc() */ @@ -1944,9 +1975,13 @@ H5O_msg_flush(H5F_t *f, H5O_t *oh, H5O_mesg_t *mesg) /* Make certain that null messages aren't in chunks w/gaps */ if (H5O_NULL_ID == msg_id) assert(oh->chunk[mesg->chunkno].gap == 0); - else + else { + /* Deleted messages should have been removed by now */ + assert(H5O_DELETED_ID != msg_id); + /* Non-null messages should always have a native pointer */ assert(mesg->native); + } #endif /* NDEBUG */ /* Encode the message itself, if it's not an "unknown" message */ @@ -1960,6 +1995,8 @@ H5O_msg_flush(H5F_t *f, H5O_t *oh, H5O_mesg_t *mesg) assert(mesg->raw_size == H5O_ALIGN_OH(oh, mesg->raw_size)); assert(mesg->raw + mesg->raw_size <= oh->chunk[mesg->chunkno].image + (oh->chunk[mesg->chunkno].size - H5O_SIZEOF_CHKSUM_OH(oh))); + assert(msg_id != H5O_DELETED_ID); + #ifndef NDEBUG /* Sanity check that the message won't overwrite past it's allocated space */ { @@ -1970,6 +2007,7 @@ H5O_msg_flush(H5F_t *f, H5O_t *oh, H5O_mesg_t *mesg) assert(msg_size <= mesg->raw_size); } #endif /* NDEBUG */ + assert(mesg->type->encode); if ((mesg->type->encode)(f, false, mesg->raw_size, mesg->raw, mesg->native) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTENCODE, FAIL, "unable to encode object header message"); diff --git a/src/H5Opkg.h b/src/H5Opkg.h index b47c82e23f9..eb22f432063 100644 --- a/src/H5Opkg.h +++ b/src/H5Opkg.h @@ -29,7 +29,7 @@ #define H5O_NCHUNKS 2 /*initial number of chunks */ #define H5O_MIN_SIZE \ 22 /* Min. obj header data size (must be big enough for a message prefix and a continuation message) */ -#define H5O_MSG_TYPES 26 /* # of types of messages */ +#define H5O_MSG_TYPES 27 /* # of types of messages */ #define H5O_MAX_CRT_ORDER_IDX 65535 /* Max. creation order index value */ /* Versions of object header structure */ @@ -303,11 +303,14 @@ struct H5O_t { unsigned min_dense; /* Minimum # of "dense" attributes */ /* Message management (stored, encoded in chunks) */ - size_t nmesgs; /*number of messages */ - size_t alloc_nmesgs; /*number of message slots */ - H5O_mesg_t *mesg; /*array of messages */ - size_t link_msgs_seen; /* # of link messages seen when loading header */ - size_t attr_msgs_seen; /* # of attribute messages seen when loading header */ + size_t nmesgs; /*number of messages */ + size_t alloc_nmesgs; /*number of message slots */ + H5O_mesg_t *mesg; /*array of messages */ + size_t link_msgs_seen; /* # of link messages seen when loading header */ + size_t attr_msgs_seen; /* # of attribute messages seen when loading header */ + unsigned mesgs_modified; /* Whether any messages were modified during this operation */ + unsigned recursion_level; /* Level of recursion within message iteration */ + unsigned num_deleted_mesgs; /* Number of deleted messages */ /* Chunk management (not stored) */ size_t nchunks; /*number of chunks */ @@ -527,6 +530,9 @@ H5_DLLVAR const H5O_msg_class_t H5O_MSG_MDCI[1]; /* Placeholder for unknown message. (0x0019) */ H5_DLLVAR const H5O_msg_class_t H5O_MSG_UNKNOWN[1]; +/* Placeholder for deleted message. (0x001a) */ +H5_DLLVAR const H5O_msg_class_t H5O_MSG_DELETED[1]; + /* * Object header "object" types */ diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index 3a1fa7c118e..74f14e01e1a 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -230,6 +230,8 @@ typedef struct H5O_copy_t { #define H5O_MDCI_MSG_ID 0x0018 /* Metadata Cache Image Message */ #define H5O_UNKNOWN_ID 0x0019 /* Placeholder message ID for unknown message. */ /* (this should never exist in a file) */ +#define H5O_DELETED_ID 0x001a /* Placeholder in mesg array in memory for a deleted message */ +/* (this should never exist in a file) */ /* * Note: Must increment H5O_MSG_TYPES in H5Opkg.h and update H5O_msg_class_g * in H5O.c when creating a new message type. Also bump the value of @@ -238,7 +240,7 @@ typedef struct H5O_copy_t { * * (this should never exist in a file) */ -#define H5O_BOGUS_INVALID_ID 0x001a /* "Bogus invalid" Message. */ +#define H5O_BOGUS_INVALID_ID 0x001b /* "Bogus invalid" Message. */ /* Shared object message types. * Shared objects can be committed, in which case the shared message contains @@ -888,7 +890,7 @@ typedef herr_t (*H5O_operator_t)(const void *mesg /*in*/, unsigned idx, void *op /* Typedef for "internal library" iteration operations */ typedef herr_t (*H5O_lib_operator_t)(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, - unsigned *oh_modified /*out*/, void *operator_data /*in,out*/); + void *operator_data /*in,out*/); /* Some syntactic sugar to make the compiler happy with two different kinds of iterator callbacks */ typedef enum H5O_mesg_operator_type_t { diff --git a/src/H5SM.c b/src/H5SM.c index 2b98bdbb98b..880eee243e6 100644 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -72,8 +72,7 @@ static herr_t H5SM__delete_from_index(H5F_t *f, H5O_t *open_oh, H5SM_index_head const H5O_shared_t *mesg, unsigned *cache_flags, size_t */*out*/ mesg_size, void **/*out*/ encoded_mesg); static herr_t H5SM__type_to_flag(unsigned type_id, unsigned *type_flag); -static herr_t H5SM__read_iter_op(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, unsigned *oh_modified, - void *_udata); +static herr_t H5SM__read_iter_op(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, void *_udata); static herr_t H5SM__read_mesg_fh_cb(const void *obj, size_t obj_len, void *_udata); static herr_t H5SM__read_mesg(H5F_t *f, const H5SM_sohm_t *mesg, H5HF_t *fheap, H5O_t *open_oh, size_t *encoding_size /*out*/, void **encoded_mesg /*out*/); @@ -2229,8 +2228,7 @@ H5SM_get_refcount(H5F_t *f, unsigned type_id, const H5O_shared_t *sh_mesg, hsize *------------------------------------------------------------------------- */ static herr_t -H5SM__read_iter_op(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, - unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/) +H5SM__read_iter_op(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, void *_udata /*in,out*/) { H5SM_read_udata_t *udata = (H5SM_read_udata_t *)_udata; herr_t ret_value = H5_ITER_CONT; diff --git a/src/H5SMmessage.c b/src/H5SMmessage.c index 230f381df2f..3d4b6c270c4 100644 --- a/src/H5SMmessage.c +++ b/src/H5SMmessage.c @@ -45,8 +45,7 @@ typedef struct H5SM_compare_udata_t { /* Local Prototypes */ /********************/ static herr_t H5SM__compare_cb(const void *obj, size_t obj_len, void *udata); -static herr_t H5SM__compare_iter_op(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, unsigned *oh_modified, - void *udata); +static herr_t H5SM__compare_iter_op(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, void *udata); /*********************/ /* Package Variables */ @@ -106,8 +105,7 @@ H5SM__compare_cb(const void *obj, size_t obj_len, void *_udata) *------------------------------------------------------------------------- */ static herr_t -H5SM__compare_iter_op(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, - unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/) +H5SM__compare_iter_op(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, void *_udata /*in,out*/) { H5SM_compare_udata_t *udata = (H5SM_compare_udata_t *)_udata; herr_t ret_value = H5_ITER_CONT; diff --git a/test/links.c b/test/links.c index 826211f1da3..ab67dc2772a 100644 --- a/test/links.c +++ b/test/links.c @@ -17536,6 +17536,211 @@ obj_exists(hid_t fapl, bool new_format) return FAIL; } /* end obj_exists() */ +/*------------------------------------------------------------------------- + * Function: delete_self_referential_link + * + * Purpose: Test deleting a hard link to the parent group. + * + * Return: Success: 0 + * Failure: -1 + *------------------------------------------------------------------------- + */ +static int +delete_self_referential_link(hid_t fapl, bool new_format) +{ + char filename[NAME_BUF_SIZE]; /* Buffer for file name */ + hid_t fid = H5I_INVALID_HID; /* File ID */ + hid_t gid = H5I_INVALID_HID; /* Group ID */ + hid_t gcpl = H5I_INVALID_HID; /* GCPL ID */ + htri_t status; /* Generic return value */ + + if (new_format) + TESTING("deleting self referential link (w/new group format)"); + else + TESTING("deleting self referential link"); + + /* Set up filename and create file*/ + h5_fixname(FILENAME[0], fapl, filename, sizeof filename); + + if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) + FAIL_STACK_ERROR; + + /* Create a group, as a destination for testing */ + if ((gid = H5Gcreate2(fid, "group", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR; + + /* Verify that H5Lexists() succeeds for hard linked object */ + if (true != H5Lexists(fid, "group", H5P_DEFAULT)) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for non-existent object in non-root group */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Create hard link on group to group */ + if (H5Lcreate_hard(gid, ".", gid, "link", H5P_DEFAULT, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for self referential link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Create second hard link on group to group */ + if (H5Lcreate_hard(gid, ".", gid, "link2", H5P_DEFAULT, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for self referential link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Delete first link */ + if (H5Ldelete(gid, "link", H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for first link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for second link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Delete second link */ + if (H5Ldelete(gid, "link2", H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for deleted link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for second link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Close group */ + if (H5Gclose(gid) < 0) + FAIL_STACK_ERROR; + gid = H5I_INVALID_HID; + + /* + * Now test deleting by creation order + */ + + /* Create GCPL with creation order tracked */ + if ((gcpl = H5Pcreate(H5P_GROUP_CREATE)) < 0) + TEST_ERROR; + if (H5Pset_link_creation_order(gcpl, H5P_CRT_ORDER_TRACKED) < 0) + TEST_ERROR; + + /* Create a group, as a destination for testing */ + if ((gid = H5Gcreate2(fid, "group2", H5P_DEFAULT, gcpl, H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR; + + /* Close GCPL */ + if (H5Pclose(gcpl) < 0) + TEST_ERROR; + gcpl = H5I_INVALID_HID; + + /* Verify that H5Lexists() succeeds for hard linked object */ + if (true != H5Lexists(fid, "group2", H5P_DEFAULT)) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for non-existent object in non-root group */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Create hard link on group2 to group2 */ + if (H5Lcreate_hard(gid, ".", gid, "link", H5P_DEFAULT, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for self referential link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Create second hard link on group to group */ + if (H5Lcreate_hard(gid, ".", gid, "link2", H5P_DEFAULT, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for self referential link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Delete first link */ + if (H5Ldelete_by_idx(gid, ".", H5_INDEX_CRT_ORDER, H5_ITER_INC, 0, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for first link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns true for second link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 1) + TEST_ERROR; + + /* Delete second link */ + if (H5Ldelete_by_idx(gid, ".", H5_INDEX_CRT_ORDER, H5_ITER_INC, 0, H5P_DEFAULT) < 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for deleted link */ + if ((status = H5Lexists(gid, "link", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Verify that H5Lexists() returns false for second link */ + if ((status = H5Lexists(gid, "link2", H5P_DEFAULT)) < 0) + TEST_ERROR; + if (status != 0) + TEST_ERROR; + + /* Close group */ + if (H5Gclose(gid) < 0) + FAIL_STACK_ERROR; + gid = H5I_INVALID_HID; + + /* Close file */ + if (H5Fclose(fid) < 0) + FAIL_STACK_ERROR; + fid = H5I_INVALID_HID; + + PASSED(); + return SUCCEED; + +error: + H5E_BEGIN_TRY + { + H5Gclose(gid); + H5Fclose(fid); + H5Pclose(gcpl); + } + H5E_END_TRY + return FAIL; +} /* end delete_self_referential_link() */ + /*------------------------------------------------------------------------- * Function: corder_create_empty * @@ -23526,6 +23731,7 @@ main(void) nerrors += obj_visit_stop(my_fapl, new_format) < 0 ? 1 : 0; nerrors += link_filters(my_fapl, new_format) < 0 ? 1 : 0; nerrors += obj_exists(my_fapl, new_format) < 0 ? 1 : 0; + nerrors += delete_self_referential_link(my_fapl, new_format) < 0 ? 1 : 0; /* Keep this test last, it's testing files that are used above */ /* do not do this for files used by external link tests */ diff --git a/test/testfiles/tbogus.h5 b/test/testfiles/tbogus.h5 index efcbd7951ae..f64229e9356 100644 Binary files a/test/testfiles/tbogus.h5 and b/test/testfiles/tbogus.h5 differ