From 5eea79eaf426ac3e51a09d3f3fe72c2b385abc89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Tue, 10 Nov 2020 11:16:00 +0100 Subject: [PATCH] Fix memory allocation We can't assume that size of a structure is a sum of sizes of its members because padding and alignment can be involved. In fact, we need to allocate more bytes for the structure than the sum of sizes of its members. The wrong assumption caused invalid writes and invalid reads which can be discovered by valgrind. Moreover, when run with MALLOC_CHECK_ environment variable set to non-zero value, the program aborted. The memory issue happened only when NDEBUG is defined, eg. when cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo or Release, it doesn't happen if cmake -DCMAKE_BUILD_TYPE=Debug which we usually use in Jenkins CI. This is most likely because in debug mode the struct SEXP contains 2 additional members which are the magic canaries and therefore is bigger. This commit wants to fix the problem by 2 step allocation in which first the size of the struct SEXP_val_lblk is used and then the array of SEXPs is allocated separately. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1891770 --- src/OVAL/probes/SEAP/_sexp-value.h | 2 +- src/OVAL/probes/SEAP/sexp-value.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/OVAL/probes/SEAP/_sexp-value.h b/src/OVAL/probes/SEAP/_sexp-value.h index 426cd2c3d..e66777ef9 100644 --- a/src/OVAL/probes/SEAP/_sexp-value.h +++ b/src/OVAL/probes/SEAP/_sexp-value.h @@ -94,7 +94,7 @@ struct SEXP_val_lblk { uintptr_t nxsz; uint16_t real; uint16_t refs; - SEXP_t memb[]; + SEXP_t *memb; }; size_t SEXP_rawval_list_length (struct SEXP_val_list *list); diff --git a/src/OVAL/probes/SEAP/sexp-value.c b/src/OVAL/probes/SEAP/sexp-value.c index a11cbc70c..b8b3ed609 100644 --- a/src/OVAL/probes/SEAP/sexp-value.c +++ b/src/OVAL/probes/SEAP/sexp-value.c @@ -106,10 +106,8 @@ uintptr_t SEXP_rawval_lblk_new (uint8_t sz) { _A(sz < 16); - struct SEXP_val_lblk *lblk = oscap_aligned_malloc( - sizeof(uintptr_t) + (2 * sizeof(uint16_t)) + (sizeof(SEXP_t) * (1 << sz)), - SEXP_LBLK_ALIGN - ); + struct SEXP_val_lblk *lblk = malloc(sizeof(struct SEXP_val_lblk)); + lblk->memb = malloc(sizeof(SEXP_t) * (1 << sz)); lblk->nxsz = ((uintptr_t)(NULL) & SEXP_LBLKP_MASK) | ((uintptr_t)sz & SEXP_LBLKS_MASK); lblk->refs = 1; @@ -519,7 +517,8 @@ void SEXP_rawval_lblk_free (uintptr_t lblkp, void (*func) (SEXP_t *)) func (lblk->memb + lblk->real); } - oscap_aligned_free(lblk); + free(lblk->memb); + free(lblk); if (next != NULL) SEXP_rawval_lblk_free ((uintptr_t)next, func); @@ -540,7 +539,8 @@ void SEXP_rawval_lblk_free1 (uintptr_t lblkp, void (*func) (SEXP_t *)) func (lblk->memb + lblk->real); } - oscap_aligned_free(lblk); + free(lblk->memb); + free(lblk); } return; -- 2.26.2