Skip to content

Commit

Permalink
Fix segfault in h5dump caused by corrupted btree node level (#5002)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bmribler
bmribler authored Oct 29, 2024
1 parent f4dbb81 commit 5bdd379
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
18 changes: 14 additions & 4 deletions src/H5B.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,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 @@ -428,6 +429,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 @@ -535,6 +537,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 @@ -792,6 +795,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 @@ -1048,7 +1052,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, 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 */
Expand Down Expand Up @@ -1078,13 +1083,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], (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)
Expand Down Expand Up @@ -1125,7 +1131,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 @@ -1191,6 +1197,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 @@ -1543,6 +1550,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 @@ -1783,6 +1791,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 @@ -1878,7 +1887,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 @@ -1924,6 +1933,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
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 != (unsigned)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
6 changes: 6 additions & 0 deletions src/H5Bpkg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -1

/****************************/
/* Package Private Typedefs */
/****************************/
Expand All @@ -60,6 +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 */
int exp_level; /* Expected level of the current node */
H5UC_t *rc_shared; /* Ref-counted shared info */
} H5B_cache_ud_t;

Expand Down

0 comments on commit 5bdd379

Please sign in to comment.