85 lines
3.0 KiB
Diff
85 lines
3.0 KiB
Diff
|
From 5eea79eaf426ac3e51a09d3f3fe72c2b385abc89 Mon Sep 17 00:00:00 2001
|
||
|
|
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
|
||
|
|
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
|
||
|
|
|