Skip to content

Commit a289461

Browse files
fortnernfortnern
authored
Fix bugs involving recursive link operations that could be triggered by deleting links to their parent group (#5853)
Fix problems that could occur when deleting an object header message inside a recursive operation during object header message traversal.
1 parent 7a920d0 commit a289461

File tree

14 files changed

+382
-75
lines changed

14 files changed

+382
-75
lines changed

‎release_docs/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,12 @@ Added Fortran wrapper h5fdsubfiling_get_file_mapping_f() for the subfiling file
523523
# 🪲 Bug Fixes
524524

525525
## Library
526+
### Fix bugs in object header operations
527+
528+
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.
529+
530+
Fixes GitHub issue #5854
531+
526532

527533
### Fixed security issues CVE-2025-6816, CVE-2025-6856 and CVE-2025-2923
528534

‎src/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ set (H5O_SOURCES
613613
${HDF5_SRC_DIR}/H5Ocopy.c
614614
${HDF5_SRC_DIR}/H5Ocopy_ref.c
615615
${HDF5_SRC_DIR}/H5Odbg.c
616+
${HDF5_SRC_DIR}/H5Odeleted.c
616617
${HDF5_SRC_DIR}/H5Odeprec.c
617618
${HDF5_SRC_DIR}/H5Odrvinfo.c
618619
${HDF5_SRC_DIR}/H5Odtype.c

‎src/H5Aint.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ typedef struct {
7474

7575
static herr_t H5A__close_cb(H5VL_object_t *attr_vol_obj, void **request);
7676
static herr_t H5A__compact_build_table_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
77-
unsigned *oh_flags_ptr, void *_udata /*in,out*/);
77+
void *_udata /*in,out*/);
7878
static herr_t H5A__dense_build_table_cb(const H5A_t *attr, void *_udata);
7979
static int H5A__attr_cmp_name_inc(const void *attr1, const void *attr2);
8080
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,
14861486
*/
14871487
static herr_t
14881488
H5A__compact_build_table_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
1489-
unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
1489+
void *_udata /*in,out*/)
14901490
{
14911491
H5A_compact_bt_ud_t *udata = (H5A_compact_bt_ud_t *)_udata; /* Operator user data */
14921492
herr_t ret_value = H5_ITER_CONT; /* Return value */

‎src/H5Oalloc.c‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -938,11 +938,11 @@ H5O__alloc_chunk(H5F_t *f, H5O_t *oh, size_t size, size_t found_null, const H5O_
938938
for (u = 0, curr_msg = &oh->mesg[0]; u < oh->nmesgs; u++, curr_msg++)
939939
if (curr_msg->chunkno == chunkno - 1) {
940940
if (curr_msg->type->id == H5O_NULL_ID) {
941-
/* Delete the null message */
942-
if (u < oh->nmesgs - 1)
943-
memmove(curr_msg, curr_msg + 1, ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t));
944-
oh->nmesgs--;
945-
} /* end if */
941+
/* Delete the null message. Defer actual deletion so we don't interfere with a higher
942+
* level of recursion by moving the messages. */
943+
curr_msg->type = H5O_MSG_DELETED;
944+
oh->num_deleted_mesgs++;
945+
}
946946
else {
947947
assert(curr_msg->type->id != H5O_CONT_ID);
948948

@@ -1037,17 +1037,10 @@ H5O__alloc_chunk(H5F_t *f, H5O_t *oh, size_t size, size_t found_null, const H5O_
10371037
/* Release any information/memory for message */
10381038
H5O__msg_free_mesg(old_null_msg);
10391039

1040-
/* Remove null message from list of messages */
1041-
if (found_msg->null_msgno < (oh->nmesgs - 1))
1042-
memmove(old_null_msg, old_null_msg + 1,
1043-
((oh->nmesgs - 1) - found_msg->null_msgno) * sizeof(H5O_mesg_t));
1044-
1045-
/* Decrement # of messages */
1046-
/* (Don't bother reducing size of message array for now -QAK) */
1047-
oh->nmesgs--;
1048-
1049-
/* Adjust message index for new NULL message */
1050-
found_null--;
1040+
/* Delete null message from list of messages. Defer actual deletion so we don't interfere with
1041+
* a higher level of recursion by moving the messages. */
1042+
old_null_msg->type = H5O_MSG_DELETED;
1043+
oh->num_deleted_mesgs++;
10511044
} /* end if */
10521045

10531046
/* Mark the new null message as dirty */
@@ -2269,6 +2262,18 @@ H5O__condense_header(H5F_t *f, H5O_t *oh)
22692262
/* check args */
22702263
assert(oh != NULL);
22712264

2265+
/* First remove all deleted messages from the object header */
2266+
for (unsigned u = 0; oh->num_deleted_mesgs > 0 && u < oh->nmesgs;)
2267+
if (oh->mesg[u].type->id == H5O_DELETED_ID) {
2268+
if (u < (oh->nmesgs - 1))
2269+
memmove(&oh->mesg[u], &oh->mesg[u + 1], ((oh->nmesgs - 1) - u) * sizeof(H5O_mesg_t));
2270+
oh->nmesgs--;
2271+
oh->num_deleted_mesgs--;
2272+
}
2273+
else
2274+
u++;
2275+
assert(oh->num_deleted_mesgs == 0);
2276+
22722277
/* Loop until no changed to the object header messages & chunks */
22732278
do {
22742279
/* Reset 'rescan chunks' flag */

‎src/H5Oattribute.c‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -109,24 +109,20 @@ typedef struct {
109109
/* Local Prototypes */
110110
/********************/
111111
static herr_t H5O__attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
112-
unsigned *oh_modified, void *_udata);
112+
void *_udata);
113113
static htri_t H5O__attr_find_opened_attr(const H5O_loc_t *loc, H5A_t **attr, const char *name_to_open);
114-
static herr_t H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence,
115-
unsigned H5_ATTR_UNUSED *oh_modified, void *_udata);
114+
static herr_t H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned sequence, void *_udata);
116115
static herr_t H5O__attr_open_by_idx_cb(const H5A_t *attr, void *_ret_attr);
117-
static herr_t H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
118-
unsigned *oh_modified, void *_udata);
116+
static herr_t H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence, void *_udata);
119117
static herr_t H5O__attr_rename_chk_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
120-
unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
121-
void *_udata);
118+
unsigned H5_ATTR_UNUSED sequence, void *_udata);
122119
static herr_t H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
123-
unsigned *oh_modified, void *_udata);
120+
void *_udata);
124121
static herr_t H5O__attr_remove_update(const H5O_loc_t *loc, H5O_t *oh, H5O_ainfo_t *ainfo);
125122
static herr_t H5O__attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg, unsigned H5_ATTR_UNUSED sequence,
126-
unsigned *oh_modified, void *_udata);
127-
static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
128-
unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
129123
void *_udata);
124+
static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
125+
unsigned H5_ATTR_UNUSED sequence, void *_udata);
130126

131127
/*********************/
132128
/* Package Variables */
@@ -152,7 +148,7 @@ static herr_t H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg,
152148
*/
153149
static herr_t
154150
H5O__attr_to_dense_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
155-
unsigned *oh_modified, void *_udata /*in,out*/)
151+
void *_udata /*in,out*/)
156152
{
157153
H5O_iter_cvt_t *udata = (H5O_iter_cvt_t *)_udata; /* Operator user data */
158154
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
178174
HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, H5_ITER_ERROR, "unable to convert into null message");
179175

180176
/* Indicate that the object header was modified */
181-
*oh_modified = H5O_MODIFY_CONDENSE;
177+
oh->mesgs_modified = H5O_MODIFY_CONDENSE;
182178

183179
done:
184180
FUNC_LEAVE_NOAPI(ret_value)
@@ -394,8 +390,7 @@ H5O__attr_create(const H5O_loc_t *loc, H5A_t *attr)
394390
*-------------------------------------------------------------------------
395391
*/
396392
static herr_t
397-
H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
398-
unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
393+
H5O__attr_open_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence, void *_udata /*in,out*/)
399394
{
400395
H5O_iter_opn_t *udata = (H5O_iter_opn_t *)_udata; /* Operator user data */
401396
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
776771
*/
777772
static herr_t
778773
H5O__attr_write_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
779-
unsigned *oh_modified, void *_udata /*in,out*/)
774+
void *_udata /*in,out*/)
780775
{
781776
H5O_iter_wrt_t *udata = (H5O_iter_wrt_t *)_udata; /* Operator user data */
782777
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
828823
"unable to update attribute in shared storage");
829824

830825
/* Indicate that the object header was modified */
831-
*oh_modified = H5O_MODIFY;
826+
oh->mesgs_modified = H5O_MODIFY;
832827

833828
/* Indicate that the attribute was found */
834829
udata->found = true;
@@ -928,8 +923,7 @@ H5O__attr_write(const H5O_loc_t *loc, H5A_t *attr)
928923
*/
929924
static herr_t
930925
H5O__attr_rename_chk_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/,
931-
unsigned H5_ATTR_UNUSED sequence, unsigned H5_ATTR_UNUSED *oh_modified,
932-
void *_udata /*in,out*/)
926+
unsigned H5_ATTR_UNUSED sequence, void *_udata /*in,out*/)
933927
{
934928
H5O_iter_ren_t *udata = (H5O_iter_ren_t *)_udata; /* Operator user data */
935929
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*/,
970964
*/
971965
static herr_t
972966
H5O__attr_rename_mod_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
973-
unsigned *oh_modified, void *_udata /*in,out*/)
967+
void *_udata /*in,out*/)
974968
{
975969
H5O_iter_ren_t *udata = (H5O_iter_ren_t *)_udata; /* Operator user data */
976970
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
10471041
HGOTO_ERROR(H5E_ATTR, H5E_CANTDELETE, H5_ITER_ERROR,
10481042
"unable to release previous attribute");
10491043

1050-
*oh_modified = H5O_MODIFY_CONDENSE;
1044+
oh->mesgs_modified = H5O_MODIFY_CONDENSE;
10511045

10521046
/* Append renamed attribute to object header */
10531047
/* (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
10651059
} /* end else */
10661060

10671061
/* Indicate that the object header was modified */
1068-
*oh_modified |= H5O_MODIFY;
1062+
oh->mesgs_modified |= H5O_MODIFY;
10691063

10701064
/* Indicate that we found an existing attribute with the old name */
10711065
udata->found = true;
@@ -1415,7 +1409,7 @@ H5O__attr_remove_update(const H5O_loc_t *loc, H5O_t *oh, H5O_ainfo_t *ainfo)
14151409
*/
14161410
static herr_t
14171411
H5O__attr_remove_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
1418-
unsigned *oh_modified, void *_udata /*in,out*/)
1412+
void *_udata /*in,out*/)
14191413
{
14201414
H5O_iter_rm_t *udata = (H5O_iter_rm_t *)_udata; /* Operator user data */
14211415
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
14341428
HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, H5_ITER_ERROR, "unable to convert into null message");
14351429

14361430
/* Indicate that the object header was modified */
1437-
*oh_modified = H5O_MODIFY_CONDENSE;
1431+
oh->mesgs_modified = H5O_MODIFY_CONDENSE;
14381432

14391433
/* Indicate that this message is the attribute to be deleted */
14401434
udata->found = true;
@@ -1674,7 +1668,7 @@ H5O__attr_count_real(H5F_t *f, H5O_t *oh, hsize_t *nattrs)
16741668
*/
16751669
static herr_t
16761670
H5O__attr_exists_cb(H5O_t H5_ATTR_UNUSED *oh, H5O_mesg_t *mesg /*in,out*/, unsigned H5_ATTR_UNUSED sequence,
1677-
unsigned H5_ATTR_UNUSED *oh_modified, void *_udata /*in,out*/)
1671+
void *_udata /*in,out*/)
16781672
{
16791673
H5O_iter_xst_t *udata = (H5O_iter_xst_t *)_udata; /* Operator user data */
16801674
herr_t ret_value = H5_ITER_CONT; /* Return value */

‎src/H5Odeleted.c‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
2+
* Copyright by The HDF Group. *
3+
* All rights reserved. *
4+
* *
5+
* This file is part of HDF5. The full HDF5 copyright notice, including *
6+
* terms governing use, modification, and redistribution, is contained in *
7+
* the LICENSE file, which can be found at the root of the source code *
8+
* distribution tree, or in https://www.hdfgroup.org/licenses. *
9+
* If you do not have access to either file, you may request a copy from *
10+
* [email protected]. *
11+
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
12+
13+
/*-------------------------------------------------------------------------
14+
*
15+
* Created: H5Odeleted.c
16+
*
17+
* Purpose: The deleted message. This should never exist in
18+
* the file.
19+
*
20+
*-------------------------------------------------------------------------
21+
*/
22+
23+
#include "H5Omodule.h" /* This source code file is part of the H5O module */
24+
25+
#include "H5private.h" /* Generic Functions */
26+
#include "H5Opkg.h" /* Object headers */
27+
28+
/* This message derives from H5O message class */
29+
const H5O_msg_class_t H5O_MSG_DELETED[1] = {{
30+
H5O_DELETED_ID, /*message id number */
31+
"deleted", /*message name for debugging */
32+
0, /*native message size */
33+
0, /* messages are shareable? */
34+
NULL, /*no decode method */
35+
NULL, /*no encode method */
36+
NULL, /*no copy method */
37+
NULL, /*no size method */
38+
NULL, /*no reset method */
39+
NULL, /*no free method */
40+
NULL, /*no file delete method */
41+
NULL, /*no link method */
42+
NULL, /*no set share method */
43+
NULL, /*no can share method */
44+
NULL, /*no pre copy native value to file */
45+
NULL, /*no copy native value to file */
46+
NULL, /*no post copy native value to file */
47+
NULL, /*no get creation index */
48+
NULL, /*no set creation index */
49+
NULL /*no debug method */
50+
}};

‎src/H5Oint.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ const H5O_msg_class_t *const H5O_msg_class_g[] = {
120120
H5O_MSG_REFCOUNT, /*0x0016 Object's ref. count */
121121
H5O_MSG_FSINFO, /*0x0017 Free-space manager info */
122122
H5O_MSG_MDCI, /*0x0018 Metadata cache image */
123-
H5O_MSG_UNKNOWN /*0x0019 Placeholder for unknown message */
123+
H5O_MSG_UNKNOWN, /*0x0019 Placeholder for unknown message */
124+
H5O_MSG_DELETED /*0x001a Placeholder for deleted message */
124125
};
125126

126127
/* Format version bounds for object header */
@@ -2918,6 +2919,8 @@ H5O__free(H5O_t *oh, bool H5_ATTR_NDEBUG_UNUSED force)
29182919
/* check args */
29192920
assert(oh);
29202921
assert(0 == oh->rc);
2922+
assert(0 == oh->mesgs_modified);
2923+
assert(0 == oh->recursion_level);
29212924

29222925
/* Destroy chunks */
29232926
if (oh->chunk) {

0 commit comments

Comments
 (0)