Skip to content

Commit

Permalink
Fix SIGABRT due to invalid heap access
Browse files Browse the repository at this point in the history
When the set update completed with "inconsistent" state, the heap is in
an invalid state and should not be accessed.

In particular, consider the case of `SAMP -> AGG_L1 -> ldms_ls`. SAMP
has a sampler that uses `LDMS_V_LIST`. AGG_L1 has finished looking up
the set from SAMP, and is in the middle of updating it. At this point,
the metadata on AGG_L1 has been populated, but the data+heap parts are
still uninitialized. In the mean time, `ldms_ls` lookup + update the set
and get SIGABRT from `ldms_heap_get()` due to bad heap signature because
of the uninitialized data+heap part on the AGG_L1 that `ldms_ls` copied
the data from.

This patch:
- adds set consistency check before getting the heap in the LDMS set
  update path.
- adds heap pointer check in the `ldms_list_first()` list/heap access.
  • Loading branch information
narategithub authored and tom95858 committed Jan 20, 2022
1 parent 4ef12e1 commit 8b36876
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 2 additions & 0 deletions ldms/src/core/ldms.c
Original file line number Diff line number Diff line change
Expand Up @@ -2642,6 +2642,8 @@ ldms_mval_t ldms_list_first(ldms_set_t s, ldms_mval_t lh, enum ldms_value_type *
ldms_mval_t le;
if (!lh->v_lh.head)
return NULL;
if (!s->heap)
return NULL;
le = ldms_heap_ptr(s->heap, lh->v_lh.head);
if (typ)
*typ = le->v_le.type;
Expand Down
5 changes: 4 additions & 1 deletion ldms/src/core/ldms_xprt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2273,7 +2273,10 @@ static void __handle_update_data(ldms_t x, struct ldms_context *ctxt,
}
if (set->meta->heap_sz) {
base = ((void*)set->data) + set->data->size - set->meta->heap_sz;
set->heap = ldms_heap_get(&set->heap_inst, &set->data->heap, base);
if (ldms_set_is_consistent(set))
set->heap = ldms_heap_get(&set->heap_inst, &set->data->heap, base);
else
set->heap = NULL;
}
ctxt->update.cb(x, set, flags, ctxt->update.cb_arg);
prev_data = data;
Expand Down

0 comments on commit 8b36876

Please sign in to comment.