Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault in h5dump caused by corrupted btree node level #5002

Merged
merged 4 commits into from
Oct 29, 2024
Merged

Fix segfault in h5dump caused by corrupted btree node level #5002

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e375d81
Re-do of PR #4862
bmribler Oct 24, 2024
2999a94
Committing clang-format changes
github-actions[bot] Oct 24, 2024
abbe91d
Fixed type of expected level
bmribler Oct 26, 2024
55a3c2c
Merge commit '2999a940eaed666381ebd2b31fec9840b117ea78' into refix_GH…
bmribler Oct 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/H5B.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -1045,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, 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 */
Expand Down Expand Up @@ -1075,13 +1080,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)
Expand Down Expand Up @@ -1122,7 +1128,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)
Expand Down Expand Up @@ -1188,6 +1194,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");

Expand Down Expand Up @@ -1540,6 +1547,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");

Expand Down Expand Up @@ -1780,6 +1788,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");

Expand Down Expand Up @@ -1875,7 +1884,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:
Expand Down Expand Up @@ -1921,6 +1930,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");

Expand Down
11 changes: 5 additions & 6 deletions src/H5Bcache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ 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");
Expand All @@ -179,12 +184,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");
Expand Down
1 change: 1 addition & 0 deletions src/H5Bpkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this an int and not unsigned since -1 is a valid value.

H5UC_t *rc_shared; /* Ref-counted shared info */
} H5B_cache_ud_t;

Expand Down
5 changes: 5 additions & 0 deletions src/H5Bprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably move to H5Bpkg.h


/* 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
Expand Down