Blob Blame History Raw
From 85a13f1f8e8d1bdbd0d5b96f543c808a1548db51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
Date: Mon, 30 Aug 2021 15:44:37 +0200
Subject: [PATCH 1/3] Lower memory limits and improve their checking

This patch attempts to mitigate problems caused by a large amount of
collected objects such as rhbz#1932833.

Specifically, these changes are made:
- Lower the threshold so that the amount of used memory is checked when
  only 1000 items are collected for the given OVAL object. That's
  because 32768 items (the original value) is already a large amount which
  occupies a lot of memory during further processing.
- Lower the memory usage ratio limit for the probe to 10 %. We have
  found experimentally that giving the probe 15 % or more will cause the
  oscap process to be killed when processing the collected data and
  generating results.
- In the calling function probe_item_collect, distinguish between return
  codes which means different behavior when there is insufficient memory
  than when the memory consumption can't be checked.
- Improve the warning message to show greater details about memory
  consumption to the user.
- Remove the check for the absolute amount of remaining free memory. As
  we can see on the example of rhbz#1932833, on systems with large
  amount of memory the remaining memory of 512 MB isn't enough memory for
  openscap to process the collected data. At the same time, if we lowered
  the usage ratio, we don't need this anymore.
- Remove useless message "spt:" from the verbose log because it's
  produced many times and pollutes the log extremely.
---
 src/OVAL/probes/probe/icache.c | 23 +++++++++++------------
 src/common/memusage.c          |  2 --
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c
index e67f5ebd3c..ccb7ee3057 100644
--- a/src/OVAL/probes/probe/icache.c
+++ b/src/OVAL/probes/probe/icache.c
@@ -457,9 +457,8 @@ int probe_icache_nop(probe_icache_t *cache)
         return (0);
 }
 
-#define PROBE_RESULT_MEMCHECK_CTRESHOLD  32768  /* item count */
-#define PROBE_RESULT_MEMCHECK_MINFREEMEM 512    /* MiB */
-#define PROBE_RESULT_MEMCHECK_MAXRATIO   0.8   /* max. memory usage ratio - used/total */
+#define PROBE_RESULT_MEMCHECK_CTRESHOLD  1000  /* item count */
+#define PROBE_RESULT_MEMCHECK_MAXRATIO   0.1   /* max. memory usage ratio - used/total */
 
 /**
  * Returns 0 if the memory constraints are not reached. Otherwise, 1 is returned.
@@ -481,18 +480,12 @@ static int probe_cobj_memcheck(size_t item_cnt)
 		c_ratio = (double)mu_proc.mu_rss/(double)(mu_sys.mu_total);
 
 		if (c_ratio > PROBE_RESULT_MEMCHECK_MAXRATIO) {
-			dW("Memory usage ratio limit reached! limit=%f, current=%f",
-			   PROBE_RESULT_MEMCHECK_MAXRATIO, c_ratio);
+			dW("Memory usage ratio limit reached! limit=%f, current=%f, used=%ld MB, free=%ld MB, total=%ld MB, count of items=%ld",
+			   PROBE_RESULT_MEMCHECK_MAXRATIO, c_ratio, mu_proc.mu_rss / 1024, mu_sys.mu_realfree / 1024, mu_sys.mu_total / 1024, item_cnt);
 			errno = ENOMEM;
 			return (1);
 		}
 
-		if ((mu_sys.mu_realfree / 1024) < PROBE_RESULT_MEMCHECK_MINFREEMEM) {
-			dW("Minimum free memory limit reached! limit=%zu, current=%zu",
-			   PROBE_RESULT_MEMCHECK_MINFREEMEM, mu_sys.mu_realfree / 1024);
-			errno = ENOMEM;
-			return (1);
-		}
 	}
 
 	return (0);
@@ -517,6 +510,7 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item)
 {
 	SEXP_t *cobj_content;
 	size_t  cobj_itemcnt;
+	int memcheck_ret;
 
 	assume_d(ctx != NULL, -1);
 	assume_d(ctx->probe_out != NULL, -1);
@@ -526,7 +520,12 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item)
 	cobj_itemcnt = SEXP_list_length(cobj_content);
 	SEXP_free(cobj_content);
 
-	if (probe_cobj_memcheck(cobj_itemcnt) != 0) {
+	memcheck_ret = probe_cobj_memcheck(cobj_itemcnt);
+	if (memcheck_ret == -1) {
+		dE("Failed to check available memory");
+		return -1;
+	}
+	if (memcheck_ret == 1) {
 
 		/*
 		 * Don't set the message again if the collected object is
diff --git a/src/common/memusage.c b/src/common/memusage.c
index af4ddb4d55..686ec9bc02 100644
--- a/src/common/memusage.c
+++ b/src/common/memusage.c
@@ -117,8 +117,6 @@ static int read_status(const char *source, void *base, struct stat_parser *spt,
 			sp = oscap_bfind(spt, spt_size, sizeof(struct stat_parser),
 			                 linebuf, (int(*)(void *, void *))&cmpkey);
 
-			dI("spt: %s", linebuf);
-
 			if (sp == NULL) {
 				/* drop end of unread line */
 				while (strchr(strval, '\n') == NULL) {

From bb26be6ffcf2a88c09e4f237516ad71db555158f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
Date: Tue, 7 Sep 2021 13:52:50 +0200
Subject: [PATCH 2/3] Allow to set memory ratio by environment variable

If the probe memory usage ratio limit will be too small or too big
in some situation, the user will be able to modify the limit easily
by setting the environment variable OSCAP_PROBE_MEMORY_USAGE_RATIO
to a different value. This can also help users when debugging memory
problems.
---
 docs/manual/manual.adoc        |  1 +
 src/OVAL/probes/probe/icache.c |  9 ++++-----
 src/OVAL/probes/probe/probe.h  |  1 +
 src/OVAL/probes/probe/worker.c | 12 ++++++++++++
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/docs/manual/manual.adoc b/docs/manual/manual.adoc
index 135426b6f0..b9ad60ced1 100644
--- a/docs/manual/manual.adoc
+++ b/docs/manual/manual.adoc
@@ -1925,6 +1925,7 @@ behaviour.
 
 * *OSCAP_FULL_VALIDATION=1* - validate all exported documents (slower)
 * *SEXP_VALIDATE_DISABLE=1* - do not validate SEXP expressions (faster)
+* *OSCAP_PROBE_MEMORY_USAGE_RATIO* - maximum memory usage ratio (used/total) for OpenSCAP probes, default: 0.1
 
 
 
diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c
index ccb7ee3057..08a0c9ad07 100644
--- a/src/OVAL/probes/probe/icache.c
+++ b/src/OVAL/probes/probe/icache.c
@@ -458,13 +458,12 @@ int probe_icache_nop(probe_icache_t *cache)
 }
 
 #define PROBE_RESULT_MEMCHECK_CTRESHOLD  1000  /* item count */
-#define PROBE_RESULT_MEMCHECK_MAXRATIO   0.1   /* max. memory usage ratio - used/total */
 
 /**
  * Returns 0 if the memory constraints are not reached. Otherwise, 1 is returned.
  * In case of an error, -1 is returned.
  */
-static int probe_cobj_memcheck(size_t item_cnt)
+static int probe_cobj_memcheck(size_t item_cnt, double max_ratio)
 {
 	if (item_cnt > PROBE_RESULT_MEMCHECK_CTRESHOLD) {
 		struct proc_memusage mu_proc;
@@ -479,9 +478,9 @@ static int probe_cobj_memcheck(size_t item_cnt)
 
 		c_ratio = (double)mu_proc.mu_rss/(double)(mu_sys.mu_total);
 
-		if (c_ratio > PROBE_RESULT_MEMCHECK_MAXRATIO) {
+		if (c_ratio > max_ratio) {
 			dW("Memory usage ratio limit reached! limit=%f, current=%f, used=%ld MB, free=%ld MB, total=%ld MB, count of items=%ld",
-			   PROBE_RESULT_MEMCHECK_MAXRATIO, c_ratio, mu_proc.mu_rss / 1024, mu_sys.mu_realfree / 1024, mu_sys.mu_total / 1024, item_cnt);
+			max_ratio, c_ratio, mu_proc.mu_rss / 1024, mu_sys.mu_realfree / 1024, mu_sys.mu_total / 1024, item_cnt);
 			errno = ENOMEM;
 			return (1);
 		}
@@ -520,7 +519,7 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item)
 	cobj_itemcnt = SEXP_list_length(cobj_content);
 	SEXP_free(cobj_content);
 
-	memcheck_ret = probe_cobj_memcheck(cobj_itemcnt);
+	memcheck_ret = probe_cobj_memcheck(cobj_itemcnt, ctx->max_mem_ratio);
 	if (memcheck_ret == -1) {
 		dE("Failed to check available memory");
 		return -1;
diff --git a/src/OVAL/probes/probe/probe.h b/src/OVAL/probes/probe/probe.h
index 0cfa234a2b..d98e741414 100644
--- a/src/OVAL/probes/probe/probe.h
+++ b/src/OVAL/probes/probe/probe.h
@@ -73,6 +73,7 @@ struct probe_ctx {
         SEXP_t         *filters;   /**< object filters (OVAL 5.8 and higher) */
         probe_icache_t *icache;    /**< item cache */
 	int offline_mode;
+	double max_mem_ratio;
 };
 
 typedef enum {
diff --git a/src/OVAL/probes/probe/worker.c b/src/OVAL/probes/probe/worker.c
index f89663544c..77f81e7885 100644
--- a/src/OVAL/probes/probe/worker.c
+++ b/src/OVAL/probes/probe/worker.c
@@ -37,6 +37,10 @@
 
 #include "worker.h"
 
+/* default max. memory usage ratio - used/total */
+/* can be overridden by environment variable OSCAP_PROBE_MEMORY_USAGE_RATIO */
+#define OSCAP_PROBE_MEMORY_USAGE_RATIO_DEFAULT 0.1
+
 extern bool  OSCAP_GSYM(varref_handling);
 extern void *OSCAP_GSYM(probe_arg);
 
@@ -924,6 +928,14 @@ SEXP_t *probe_worker(probe_t *probe, SEAP_msg_t *msg_in, int *ret)
 
 		pctx.offline_mode = probe->selected_offline_mode;
 
+		pctx.max_mem_ratio = OSCAP_PROBE_MEMORY_USAGE_RATIO_DEFAULT;
+		char *max_ratio_str = getenv("OSCAP_PROBE_MEMORY_USAGE_RATIO");
+		if (max_ratio_str != NULL) {
+			double max_ratio = strtod(max_ratio_str, NULL);
+			if (max_ratio != 0)
+				pctx.max_mem_ratio = max_ratio;
+		}
+
 		/* simple object */
                 pctx.icache  = probe->icache;
 		pctx.filters = probe_prepare_filters(probe, probe_in);

From 521756ac0668e0c686798db957f8c06ef7d55085 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
Date: Wed, 15 Sep 2021 14:41:30 +0200
Subject: [PATCH 3/3] Update src/OVAL/probes/probe/worker.c

Co-authored-by: Evgeny Kolesnikov <evgenyz@gmail.com>
---
 src/OVAL/probes/probe/worker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/OVAL/probes/probe/worker.c b/src/OVAL/probes/probe/worker.c
index 77f81e7885..9b920edc15 100644
--- a/src/OVAL/probes/probe/worker.c
+++ b/src/OVAL/probes/probe/worker.c
@@ -932,7 +932,7 @@ SEXP_t *probe_worker(probe_t *probe, SEAP_msg_t *msg_in, int *ret)
 		char *max_ratio_str = getenv("OSCAP_PROBE_MEMORY_USAGE_RATIO");
 		if (max_ratio_str != NULL) {
 			double max_ratio = strtod(max_ratio_str, NULL);
-			if (max_ratio != 0)
+			if (max_ratio > 0)
 				pctx.max_mem_ratio = max_ratio;
 		}