From e375d815aa16e3a8ffe4a9480619a4b12725bef5 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Thu, 24 Oct 2024 11:53:39 -0400 Subject: [PATCH 1/3] Re-do of PR #4862 Added another argument, expected node level, to H5B__iterate_helper to pass down to H5B__cache_deserialize for checking the decoded node level. When this expected level is not known, the new macro H5_UNKNOWN_NODELEVEL (-1) will be used for not checking the level. Fixes GH-4432 --- src/H5B.c | 17 +++++++++++++---- src/H5Bcache.c | 12 ++++++------ src/H5Bpkg.h | 1 + src/H5Bprivate.h | 5 +++++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/H5B.c b/src/H5B.c index 9c081a91fe0..1c5e13e8fb5 100644 --- a/src/H5B.c +++ b/src/H5B.c @@ -306,6 +306,7 @@ H5B_find(H5F_t *f, const H5B_class_t *type, haddr_t addr, bool *found, void *uda cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to load B-tree node"); @@ -425,6 +426,7 @@ H5B__split(H5F_t *f, H5B_ins_ud_t *bt_ud, unsigned idx, void *udata, H5B_ins_ud_ cache_udata.f = f; cache_udata.type = shared->type; cache_udata.rc_shared = bt_ud->bt->rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (split_bt_ud->bt = (H5B_t *)H5AC_protect(f, H5AC_BT, split_bt_ud->addr, &cache_udata, H5AC__NO_FLAGS_SET))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to protect B-tree"); @@ -532,6 +534,7 @@ H5B_insert(H5F_t *f, const H5B_class_t *type, haddr_t addr, void *udata) cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; bt_ud.addr = addr; if (NULL == (bt_ud.bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__NO_FLAGS_SET))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to locate root of B-tree"); @@ -789,6 +792,7 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8 cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (0 == bt->nchildren) { /* @@ -1045,7 +1049,7 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8 *------------------------------------------------------------------------- */ static herr_t -H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, H5B_operator_t op, void *udata) +H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned exp_level, H5B_operator_t op, void *udata) { H5B_t *bt = NULL; /* Pointer to current B-tree node */ H5UC_t *rc_shared; /* Ref-counted shared info */ @@ -1075,13 +1079,14 @@ H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, H5B_operato cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = exp_level; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, H5_ITER_ERROR, "unable to load B-tree node"); /* Iterate over node's children */ for (u = 0; u < bt->nchildren && ret_value == H5_ITER_CONT; u++) { if (bt->level > 0) - ret_value = H5B__iterate_helper(f, type, bt->child[u], op, udata); + ret_value = H5B__iterate_helper(f, type, bt->child[u], bt->level-1, op, udata); else ret_value = (*op)(f, H5B_NKEY(bt, shared, u), bt->child[u], H5B_NKEY(bt, shared, u + 1), udata); if (ret_value < 0) @@ -1122,7 +1127,7 @@ H5B_iterate(H5F_t *f, const H5B_class_t *type, haddr_t addr, H5B_operator_t op, assert(udata); /* Iterate over the B-tree records */ - if ((ret_value = H5B__iterate_helper(f, type, addr, op, udata)) < 0) + if ((ret_value = H5B__iterate_helper(f, type, addr, H5B_UNKNOWN_NODELEVEL, op, udata)) < 0) HERROR(H5E_BTREE, H5E_BADITER, "B-tree iteration failed"); FUNC_LEAVE_NOAPI(ret_value) @@ -1188,6 +1193,7 @@ H5B__remove_helper(H5F_t *f, haddr_t addr, const H5B_class_t *type, int level, u cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__NO_FLAGS_SET))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, H5B_INS_ERROR, "unable to load B-tree node"); @@ -1540,6 +1546,7 @@ H5B_delete(H5F_t *f, const H5B_class_t *type, haddr_t addr, void *udata) cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__NO_FLAGS_SET))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to load B-tree node"); @@ -1780,6 +1787,7 @@ H5B__get_info_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, const H5B_ cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to load B-tree node"); @@ -1875,7 +1883,7 @@ H5B_get_info(H5F_t *f, const H5B_class_t *type, haddr_t addr, H5B_info_t *bt_inf /* Iterate over the B-tree records, making any "leaf" callbacks */ /* (Only if operator defined) */ if (op) - if ((ret_value = H5B__iterate_helper(f, type, addr, op, udata)) < 0) + if ((ret_value = H5B__iterate_helper(f, type, addr, H5B_UNKNOWN_NODELEVEL, op, udata)) < 0) HERROR(H5E_BTREE, H5E_BADITER, "B-tree iteration failed"); done: @@ -1921,6 +1929,7 @@ H5B_valid(H5F_t *f, const H5B_class_t *type, haddr_t addr) cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; + cache_udata.exp_level = H5B_UNKNOWN_NODELEVEL; if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG))) HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "unable to protect B-tree node"); diff --git a/src/H5Bcache.c b/src/H5Bcache.c index 8d3655dadff..78f87bce7e3 100644 --- a/src/H5Bcache.c +++ b/src/H5Bcache.c @@ -170,6 +170,12 @@ H5B__cache_deserialize(const void *_image, size_t len, void *_udata, bool H5_ATT HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, NULL, "incorrect B-tree node type"); bt->level = *image++; + /* Check in case of level is corrupted, if expected level is known */ + if (udata->exp_level != H5B_UNKNOWN_NODELEVEL) + if (bt->level != udata->exp_level) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, + "level is not as expected, possibly corrupted"); + /* Entries used */ if (H5_IS_BUFFER_OVERFLOW(image, 2, p_end)) HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); @@ -179,12 +185,6 @@ H5B__cache_deserialize(const void *_image, size_t len, void *_udata, bool H5_ATT if (bt->nchildren > shared->two_k) HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, "number of children is greater than maximum"); - /* Check in case of level is corrupted, it is unreasonable for level to be - larger than the number of entries */ - if (bt->level > bt->nchildren) - HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, - "level cannot be greater than the number of children, possibly corrupted"); - /* Sibling pointers */ if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f), p_end)) HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); diff --git a/src/H5Bpkg.h b/src/H5Bpkg.h index 4f61614a9e0..51117f48d97 100644 --- a/src/H5Bpkg.h +++ b/src/H5Bpkg.h @@ -60,6 +60,7 @@ typedef struct H5B_t { typedef struct H5B_cache_ud_t { H5F_t *f; /* File that B-tree node is within */ const struct H5B_class_t *type; /* Type of tree */ + unsigned exp_level; /* Expected level of the current node */ H5UC_t *rc_shared; /* Ref-counted shared info */ } H5B_cache_ud_t; diff --git a/src/H5Bprivate.h b/src/H5Bprivate.h index 22bcaf53aee..c35b1891e87 100644 --- a/src/H5Bprivate.h +++ b/src/H5Bprivate.h @@ -35,6 +35,11 @@ /* Library Private Typedefs */ /****************************/ +/* Indicates that the level of the current node is unknown. When the level + * is known, it can be used to detect corrupted level during decoding + */ +#define H5B_UNKNOWN_NODELEVEL (unsigned)-1 + /* B-tree IDs for various internal things. */ /* Note - if more of these are added, any 'K' values (for internal or leaf * nodes) they use will need to be stored in the file somewhere. -QAK From 2999a940eaed666381ebd2b31fec9840b117ea78 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:56:33 +0000 Subject: [PATCH 2/3] Committing clang-format changes --- src/H5B.c | 5 +++-- src/H5Bcache.c | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/H5B.c b/src/H5B.c index 1c5e13e8fb5..85b66f25e51 100644 --- a/src/H5B.c +++ b/src/H5B.c @@ -1049,7 +1049,8 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8 *------------------------------------------------------------------------- */ static herr_t -H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned exp_level, H5B_operator_t op, void *udata) +H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned exp_level, H5B_operator_t op, + void *udata) { H5B_t *bt = NULL; /* Pointer to current B-tree node */ H5UC_t *rc_shared; /* Ref-counted shared info */ @@ -1086,7 +1087,7 @@ H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned ex /* Iterate over node's children */ for (u = 0; u < bt->nchildren && ret_value == H5_ITER_CONT; u++) { if (bt->level > 0) - ret_value = H5B__iterate_helper(f, type, bt->child[u], bt->level-1, op, udata); + ret_value = H5B__iterate_helper(f, type, bt->child[u], bt->level - 1, op, udata); else ret_value = (*op)(f, H5B_NKEY(bt, shared, u), bt->child[u], H5B_NKEY(bt, shared, u + 1), udata); if (ret_value < 0) diff --git a/src/H5Bcache.c b/src/H5Bcache.c index 78f87bce7e3..8c4810af897 100644 --- a/src/H5Bcache.c +++ b/src/H5Bcache.c @@ -173,8 +173,7 @@ H5B__cache_deserialize(const void *_image, size_t len, void *_udata, bool H5_ATT /* Check in case of level is corrupted, if expected level is known */ if (udata->exp_level != H5B_UNKNOWN_NODELEVEL) if (bt->level != udata->exp_level) - HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, - "level is not as expected, possibly corrupted"); + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, "level is not as expected, possibly corrupted"); /* Entries used */ if (H5_IS_BUFFER_OVERFLOW(image, 2, p_end)) From abbe91dee4f44ebf9c7b2eb28bb3b611474e430d Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Sat, 26 Oct 2024 03:17:39 -0400 Subject: [PATCH 3/3] Fixed type of expected level --- src/H5B.c | 4 ++-- src/H5Bcache.c | 2 +- src/H5Bpkg.h | 7 ++++++- src/H5Bprivate.h | 5 ----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/H5B.c b/src/H5B.c index 1c5e13e8fb5..22ce04c2580 100644 --- a/src/H5B.c +++ b/src/H5B.c @@ -1049,7 +1049,7 @@ H5B__insert_helper(H5F_t *f, H5B_ins_ud_t *bt_ud, const H5B_class_t *type, uint8 *------------------------------------------------------------------------- */ static herr_t -H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned exp_level, H5B_operator_t op, void *udata) +H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, int exp_level, H5B_operator_t op, void *udata) { H5B_t *bt = NULL; /* Pointer to current B-tree node */ H5UC_t *rc_shared; /* Ref-counted shared info */ @@ -1086,7 +1086,7 @@ H5B__iterate_helper(H5F_t *f, const H5B_class_t *type, haddr_t addr, unsigned ex /* Iterate over node's children */ for (u = 0; u < bt->nchildren && ret_value == H5_ITER_CONT; u++) { if (bt->level > 0) - ret_value = H5B__iterate_helper(f, type, bt->child[u], bt->level-1, op, udata); + ret_value = H5B__iterate_helper(f, type, bt->child[u], (int)bt->level-1, op, udata); else ret_value = (*op)(f, H5B_NKEY(bt, shared, u), bt->child[u], H5B_NKEY(bt, shared, u + 1), udata); if (ret_value < 0) diff --git a/src/H5Bcache.c b/src/H5Bcache.c index 78f87bce7e3..e6653e03577 100644 --- a/src/H5Bcache.c +++ b/src/H5Bcache.c @@ -172,7 +172,7 @@ H5B__cache_deserialize(const void *_image, size_t len, void *_udata, bool H5_ATT /* Check in case of level is corrupted, if expected level is known */ if (udata->exp_level != H5B_UNKNOWN_NODELEVEL) - if (bt->level != udata->exp_level) + if (bt->level != (unsigned)udata->exp_level) HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, "level is not as expected, possibly corrupted"); diff --git a/src/H5Bpkg.h b/src/H5Bpkg.h index 51117f48d97..169b8c99667 100644 --- a/src/H5Bpkg.h +++ b/src/H5Bpkg.h @@ -39,6 +39,11 @@ /* # of bits for node level: 1 byte */ #define LEVEL_BITS 8 +/* Indicates that the level of the current node is unknown. When the level + * is known, it can be used to detect corrupted level during decoding + */ +#define H5B_UNKNOWN_NODELEVEL (int)-1 + /****************************/ /* Package Private Typedefs */ /****************************/ @@ -60,7 +65,7 @@ typedef struct H5B_t { typedef struct H5B_cache_ud_t { H5F_t *f; /* File that B-tree node is within */ const struct H5B_class_t *type; /* Type of tree */ - unsigned exp_level; /* Expected level of the current node */ + int exp_level; /* Expected level of the current node */ H5UC_t *rc_shared; /* Ref-counted shared info */ } H5B_cache_ud_t; diff --git a/src/H5Bprivate.h b/src/H5Bprivate.h index c35b1891e87..22bcaf53aee 100644 --- a/src/H5Bprivate.h +++ b/src/H5Bprivate.h @@ -35,11 +35,6 @@ /* Library Private Typedefs */ /****************************/ -/* Indicates that the level of the current node is unknown. When the level - * is known, it can be used to detect corrupted level during decoding - */ -#define H5B_UNKNOWN_NODELEVEL (unsigned)-1 - /* B-tree IDs for various internal things. */ /* Note - if more of these are added, any 'K' values (for internal or leaf * nodes) they use will need to be stored in the file somewhere. -QAK