From d1045b2bc1c901dab3fc5b085e3170e8f08a527a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Mon, 13 Nov 2023 14:50:00 +0100 Subject: [PATCH 1/8] Refactor: Extract function This commit extracts the code that marks the item produced by probe as incomplete to a static function _mark_collected_object_as_incomplete so that it can be reused in other code later --- src/OVAL/probes/probe/icache.c | 49 +++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c index 6a18b62af5..93113da192 100644 --- a/src/OVAL/probes/probe/icache.c +++ b/src/OVAL/probes/probe/icache.c @@ -528,6 +528,31 @@ static int probe_cobj_memcheck(size_t item_cnt, double max_ratio) return (0); } +static int _mark_collected_object_as_incomplete(struct probe_ctx *ctx, const char *message) +{ + + /* + * Don't set the message again if the collected object is + * already flagged as incomplete. + */ + if (probe_cobj_get_flag(ctx->probe_out) == SYSCHAR_FLAG_INCOMPLETE) { + return 0; + } + /* + * Sync with the icache thread before modifying the + * collected object. + */ + if (probe_icache_nop(ctx->icache) != 0) { + return -1; + } + + SEXP_t *sexp_msg = probe_msg_creat(OVAL_MESSAGE_LEVEL_WARNING, (char *) message); + probe_cobj_add_msg(ctx->probe_out, sexp_msg); + probe_cobj_set_flag(ctx->probe_out, SYSCHAR_FLAG_INCOMPLETE); + SEXP_free(sexp_msg); + return 0; +} + /** * Collect an item * This function adds an item the collected object assosiated @@ -565,27 +590,9 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item) } if (memcheck_ret == 1) { SEXP_free(item); - - /* - * Don't set the message again if the collected object is - * already flagged as incomplete. - */ - if (probe_cobj_get_flag(ctx->probe_out) != SYSCHAR_FLAG_INCOMPLETE) { - SEXP_t *msg; - /* - * Sync with the icache thread before modifying the - * collected object. - */ - if (probe_icache_nop(ctx->icache) != 0) - return -1; - - msg = probe_msg_creat(OVAL_MESSAGE_LEVEL_WARNING, - "Object is incomplete due to memory constraints."); - - probe_cobj_add_msg(ctx->probe_out, msg); - probe_cobj_set_flag(ctx->probe_out, SYSCHAR_FLAG_INCOMPLETE); - - SEXP_free(msg); + const char *message = "Object is incomplete due to memory constraints."; + if (_mark_collected_object_as_incomplete(ctx, message) != 0) { + return -1; } return 2; From b20fca6e0dc3840fd537834e4a63540a397849d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Mon, 13 Nov 2023 15:44:11 +0100 Subject: [PATCH 2/8] Introduce a limit for collected objects If a single OVAL object matches more than 10000 items, we will stop collecting these items and mark the object as incomplete. This should prevent some situations in which the produced results are too large and cause memory problems or inability to generate the HTML report. --- src/OVAL/probes/probe/icache.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c index 93113da192..46ca3ec573 100644 --- a/src/OVAL/probes/probe/icache.c +++ b/src/OVAL/probes/probe/icache.c @@ -39,11 +39,14 @@ #include "probe-api.h" #include "common/debug_priv.h" #include "common/memusage.h" +#include "oscap_helpers.h" #include "probe.h" #include "icache.h" #include "_sexp-ID.h" +#define PROBE_ITEM_COLLECT_MAX 1000 + static volatile uint32_t next_ID = 0; #if !defined(HAVE_ATOMIC_FUNCTIONS) @@ -582,6 +585,16 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item) cobj_itemcnt = SEXP_list_length(cobj_content); SEXP_free(cobj_content); + if (cobj_itemcnt >= PROBE_ITEM_COLLECT_MAX) { + char *message = oscap_sprintf("Object is incomplete because the object matches more than %d items.", PROBE_ITEM_COLLECT_MAX); + if (_mark_collected_object_as_incomplete(ctx, message) != 0) { + free(message); + return -1; + } + free(message); + return 2; + } + memcheck_ret = probe_cobj_memcheck(cobj_itemcnt, ctx->max_mem_ratio); if (memcheck_ret == -1) { dE("Failed to check available memory"); From 5e3d8bc806cc34ce28693149eac5f50f93ca838d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Tue, 14 Nov 2023 12:59:40 +0100 Subject: [PATCH 3/8] Test collected items limit This commit adds a simple test that covers a situation when the probe collects more items than the limit for a single OVAL object. --- tests/CMakeLists.txt | 1 + tests/memory/CMakeLists.txt | 1 + tests/memory/collect_limit.oval.xml | 37 +++++++++++++++++++++++++++++ tests/memory/collect_limit.sh | 18 ++++++++++++++ 4 files changed, 57 insertions(+) create mode 100644 tests/memory/CMakeLists.txt create mode 100644 tests/memory/collect_limit.oval.xml create mode 100755 tests/memory/collect_limit.sh diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a614a7073e..44a811a285 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -30,6 +30,7 @@ add_subdirectory("curl") add_subdirectory("CPE") add_subdirectory("DS") add_subdirectory("mitre") +add_subdirectory("memory") add_subdirectory("nist") add_subdirectory("oscap_string") add_subdirectory("oval_details") diff --git a/tests/memory/CMakeLists.txt b/tests/memory/CMakeLists.txt new file mode 100644 index 0000000000..6329c1599e --- /dev/null +++ b/tests/memory/CMakeLists.txt @@ -0,0 +1 @@ +add_oscap_test("collect_limit.sh") diff --git a/tests/memory/collect_limit.oval.xml b/tests/memory/collect_limit.oval.xml new file mode 100644 index 0000000000..63e4b4b535 --- /dev/null +++ b/tests/memory/collect_limit.oval.xml @@ -0,0 +1,37 @@ + + + + 5.10.1 + 0001-01-01T00:00:00+00:00 + + + + + + x + x + + x + + + + + + + + + + + + + + + + + /tmp/longfile + ^.*$ + 1 + + + + diff --git a/tests/memory/collect_limit.sh b/tests/memory/collect_limit.sh new file mode 100755 index 0000000000..4767666f69 --- /dev/null +++ b/tests/memory/collect_limit.sh @@ -0,0 +1,18 @@ +#!/bin/bash + +set -e -o pipefail + +. $builddir/tests/test_common.sh + +# PROBE_ITEM_COLLECT_MAX limit is 1000 +seq 1010 > /tmp/longfile +result=$(mktemp) +$OSCAP oval eval --results "$result" $srcdir/collect_limit.oval.xml +assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object' +assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object[@flag="incomplete"]' +assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object/message[@level="warning"]' +text="Object is incomplete because the object matches more than 1000 items." +assert_exists 1 "/oval_results/results/system/oval_system_characteristics/collected_objects/object/message[text()=\"$text\"]" +assert_exists 1000 '/oval_results/results/system/oval_system_characteristics/system_data/ind-sys:textfilecontent_item' +rm -f /tmp/longfile +rm -f "$result" From 176f1a8eb7bfef07b07d9fd3e1642db5ebc5fa4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Tue, 14 Nov 2023 11:39:38 +0100 Subject: [PATCH 4/8] Handle return value of probe_item_collect The `probe_item_collect` function can fail and it can return various return codes. But, the return codes aren't handled in the caller function `process_file`. That means the `process_file` continues to process items even if no other items can be collected due to memory constraints or other problems. With this commit, we will read the return code of `probe_item_collect` and react accordingly. --- src/OVAL/probes/independent/textfilecontent54_probe.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/OVAL/probes/independent/textfilecontent54_probe.c b/src/OVAL/probes/independent/textfilecontent54_probe.c index d3a5c6eb22..4c6c9a1fa1 100644 --- a/src/OVAL/probes/independent/textfilecontent54_probe.c +++ b/src/OVAL/probes/independent/textfilecontent54_probe.c @@ -240,11 +240,14 @@ static int process_file(const char *prefix, const char *path, const char *file, item = create_item(path, file, pfd->pattern, cur_inst, substrs, substr_cnt, over); - probe_item_collect(pfd->ctx, item); - for (k = 0; k < substr_cnt; ++k) free(substrs[k]); free(substrs); + int pic_ret = probe_item_collect(pfd->ctx, item); + if (pic_ret == 2 || pic_ret == -1) { + ret = -4; + break; + } } } } while (substr_cnt > 0 && ofs < buf_used); From f2461c7498827fbc087ec7dad238e0bb87acecf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Tue, 21 Nov 2023 16:57:03 +0100 Subject: [PATCH 5/8] Make probe items limit configurable Users can limit the amount of items collected by setting the `OSCAP_PROBE_MAX_COLLECTED_ITEMS` environment variable. By default, the amount of items is unlimited. --- docs/manual/manual.adoc | 1 + src/OVAL/probes/probe/icache.c | 6 ++---- src/OVAL/probes/probe/probe.h | 6 ++++++ src/OVAL/probes/probe/worker.c | 12 ++++++++---- tests/memory/collect_limit.sh | 8 ++++---- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/docs/manual/manual.adoc b/docs/manual/manual.adoc index e84babe421..ab069a3708 100644 --- a/docs/manual/manual.adoc +++ b/docs/manual/manual.adoc @@ -1618,6 +1618,7 @@ not considered local by the scanner: * `SEXP_VALIDATE_DISABLE` - If set, `oscap` will not validate SEXP expressions during its execution. * `SOURCE_DATE_EPOCH` - Timestamp in seconds since epoch. This timestamp will be used instead of the current time to populate `timestamp` attributes in SCAP source data streams created by `oscap ds sds-compose` sub-module. This is used for reproducible builds of data streams. * `OSCAP_PROBE_MEMORY_USAGE_RATIO` - maximum memory usage ratio (used/total) for OpenSCAP probes, default: 0.1 +* `OSCAP_PROBE_MAX_COLLECTED_ITEMS` - maximal count of collected items by OpenSCAP probe for a single OVAL object evaluation Also, OpenSCAP uses `libcurl` library which also can be configured using environment variables. See https://curl.se/libcurl/c/libcurl-env.html[the list of libcurl environment variables]. diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c index 46ca3ec573..cd86b13119 100644 --- a/src/OVAL/probes/probe/icache.c +++ b/src/OVAL/probes/probe/icache.c @@ -45,8 +45,6 @@ #include "icache.h" #include "_sexp-ID.h" -#define PROBE_ITEM_COLLECT_MAX 1000 - static volatile uint32_t next_ID = 0; #if !defined(HAVE_ATOMIC_FUNCTIONS) @@ -585,8 +583,8 @@ int probe_item_collect(struct probe_ctx *ctx, SEXP_t *item) cobj_itemcnt = SEXP_list_length(cobj_content); SEXP_free(cobj_content); - if (cobj_itemcnt >= PROBE_ITEM_COLLECT_MAX) { - char *message = oscap_sprintf("Object is incomplete because the object matches more than %d items.", PROBE_ITEM_COLLECT_MAX); + if (ctx->max_collected_items != OSCAP_PROBE_COLLECT_UNLIMITED && cobj_itemcnt >= ctx->max_collected_items) { + char *message = oscap_sprintf("Object is incomplete because the object matches more than %ld items.", ctx->max_collected_items); if (_mark_collected_object_as_incomplete(ctx, message) != 0) { free(message); return -1; diff --git a/src/OVAL/probes/probe/probe.h b/src/OVAL/probes/probe/probe.h index d3a488c4d5..3b53a4a233 100644 --- a/src/OVAL/probes/probe/probe.h +++ b/src/OVAL/probes/probe/probe.h @@ -43,6 +43,11 @@ #include "common/util.h" #include "common/compat_pthread_barrier.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.33 +#define OSCAP_PROBE_COLLECT_UNLIMITED 0 + typedef struct { pthread_rwlock_t rwlock; uint32_t flags; @@ -84,6 +89,7 @@ struct probe_ctx { probe_icache_t *icache; /**< item cache */ int offline_mode; double max_mem_ratio; + size_t max_collected_items; }; typedef enum { diff --git a/src/OVAL/probes/probe/worker.c b/src/OVAL/probes/probe/worker.c index fddc1b3965..526bbbe78e 100644 --- a/src/OVAL/probes/probe/worker.c +++ b/src/OVAL/probes/probe/worker.c @@ -52,10 +52,6 @@ extern int chroot(const char *); #include "probe-table.h" #include "probe.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.33 - extern bool OSCAP_GSYM(varref_handling); extern void *OSCAP_GSYM(probe_arg); @@ -1078,6 +1074,14 @@ SEXP_t *probe_worker(probe_t *probe, SEAP_msg_t *msg_in, int *ret) if (max_ratio > 0) pctx.max_mem_ratio = max_ratio; } + pctx.max_collected_items = OSCAP_PROBE_COLLECT_UNLIMITED; + char *max_collected_items_str = getenv("OSCAP_PROBE_MAX_COLLECTED_ITEMS"); + if (max_collected_items_str != NULL) { + int max_collected_items = strtol(max_collected_items_str, NULL, 0); + if (max_collected_items > 0) { + pctx.max_collected_items = max_collected_items; + } + } /* simple object */ pctx.icache = probe->icache; diff --git a/tests/memory/collect_limit.sh b/tests/memory/collect_limit.sh index 4767666f69..d3c43b8aee 100755 --- a/tests/memory/collect_limit.sh +++ b/tests/memory/collect_limit.sh @@ -4,15 +4,15 @@ set -e -o pipefail . $builddir/tests/test_common.sh -# PROBE_ITEM_COLLECT_MAX limit is 1000 -seq 1010 > /tmp/longfile +export OSCAP_PROBE_MAX_COLLECTED_ITEMS=100 +seq 110 > /tmp/longfile result=$(mktemp) $OSCAP oval eval --results "$result" $srcdir/collect_limit.oval.xml assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object' assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object[@flag="incomplete"]' assert_exists 1 '/oval_results/results/system/oval_system_characteristics/collected_objects/object/message[@level="warning"]' -text="Object is incomplete because the object matches more than 1000 items." +text="Object is incomplete because the object matches more than 100 items." assert_exists 1 "/oval_results/results/system/oval_system_characteristics/collected_objects/object/message[text()=\"$text\"]" -assert_exists 1000 '/oval_results/results/system/oval_system_characteristics/system_data/ind-sys:textfilecontent_item' +assert_exists 100 '/oval_results/results/system/oval_system_characteristics/system_data/ind-sys:textfilecontent_item' rm -f /tmp/longfile rm -f "$result" From fb97a5135e75d883afffb73aede47ad3610dbecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Thu, 23 Nov 2023 14:47:44 +0100 Subject: [PATCH 6/8] Add comment for visual consistency --- src/OVAL/probes/probe/probe.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OVAL/probes/probe/probe.h b/src/OVAL/probes/probe/probe.h index 3b53a4a233..20244fab27 100644 --- a/src/OVAL/probes/probe/probe.h +++ b/src/OVAL/probes/probe/probe.h @@ -46,6 +46,10 @@ /* 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.33 + +/* By default, probes can collect unlimited amount of items. Ths behavior can + * be overridden by environment variable OSCAP_PROBE_MAX_COLLECTED_ITEMS. + */ #define OSCAP_PROBE_COLLECT_UNLIMITED 0 typedef struct { From dd1977c8464dff740ddb3b1494274d5468175a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Mon, 4 Dec 2023 09:12:35 +0100 Subject: [PATCH 7/8] Remove empty line --- src/OVAL/probes/probe/icache.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OVAL/probes/probe/icache.c b/src/OVAL/probes/probe/icache.c index cd86b13119..45936a892a 100644 --- a/src/OVAL/probes/probe/icache.c +++ b/src/OVAL/probes/probe/icache.c @@ -531,7 +531,6 @@ static int probe_cobj_memcheck(size_t item_cnt, double max_ratio) static int _mark_collected_object_as_incomplete(struct probe_ctx *ctx, const char *message) { - /* * Don't set the message again if the collected object is * already flagged as incomplete. From 6b29c041be0e0f60f96516e6fee7bb5664ea65dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Mon, 4 Dec 2023 09:22:22 +0100 Subject: [PATCH 8/8] Rename "memory" directory to "probe_behavior" The behavior alteration we are testing here is not exactly connected to memory. I'd rather name it like probe_behavior to make the folder a collection of tests related to these env. vars. --- tests/CMakeLists.txt | 2 +- tests/{memory => probe_behavior}/CMakeLists.txt | 0 tests/{memory => probe_behavior}/cgroup_test.sh | 0 .../collect_limit.oval.xml | 0 tests/{memory => probe_behavior}/collect_limit.sh | 0 .../{memory => probe_behavior}/ssg-rhel7-ds.xml.bz2 | Bin .../ssg-rhel7-oom-tailoring.xml | 0 .../{memory => probe_behavior}/ssg-rhel8-ds.xml.bz2 | Bin .../ssg-rhel8-oom-tailoring.xml | 0 9 files changed, 1 insertion(+), 1 deletion(-) rename tests/{memory => probe_behavior}/CMakeLists.txt (100%) rename tests/{memory => probe_behavior}/cgroup_test.sh (100%) rename tests/{memory => probe_behavior}/collect_limit.oval.xml (100%) rename tests/{memory => probe_behavior}/collect_limit.sh (100%) rename tests/{memory => probe_behavior}/ssg-rhel7-ds.xml.bz2 (100%) rename tests/{memory => probe_behavior}/ssg-rhel7-oom-tailoring.xml (100%) rename tests/{memory => probe_behavior}/ssg-rhel8-ds.xml.bz2 (100%) rename tests/{memory => probe_behavior}/ssg-rhel8-oom-tailoring.xml (100%) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 44a811a285..d4bfc7dcbe 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -30,10 +30,10 @@ add_subdirectory("curl") add_subdirectory("CPE") add_subdirectory("DS") add_subdirectory("mitre") -add_subdirectory("memory") add_subdirectory("nist") add_subdirectory("oscap_string") add_subdirectory("oval_details") +add_subdirectory("probe_behavior") add_subdirectory("probes") add_subdirectory("report") add_subdirectory("sce") diff --git a/tests/memory/CMakeLists.txt b/tests/probe_behavior/CMakeLists.txt similarity index 100% rename from tests/memory/CMakeLists.txt rename to tests/probe_behavior/CMakeLists.txt diff --git a/tests/memory/cgroup_test.sh b/tests/probe_behavior/cgroup_test.sh similarity index 100% rename from tests/memory/cgroup_test.sh rename to tests/probe_behavior/cgroup_test.sh diff --git a/tests/memory/collect_limit.oval.xml b/tests/probe_behavior/collect_limit.oval.xml similarity index 100% rename from tests/memory/collect_limit.oval.xml rename to tests/probe_behavior/collect_limit.oval.xml diff --git a/tests/memory/collect_limit.sh b/tests/probe_behavior/collect_limit.sh similarity index 100% rename from tests/memory/collect_limit.sh rename to tests/probe_behavior/collect_limit.sh diff --git a/tests/memory/ssg-rhel7-ds.xml.bz2 b/tests/probe_behavior/ssg-rhel7-ds.xml.bz2 similarity index 100% rename from tests/memory/ssg-rhel7-ds.xml.bz2 rename to tests/probe_behavior/ssg-rhel7-ds.xml.bz2 diff --git a/tests/memory/ssg-rhel7-oom-tailoring.xml b/tests/probe_behavior/ssg-rhel7-oom-tailoring.xml similarity index 100% rename from tests/memory/ssg-rhel7-oom-tailoring.xml rename to tests/probe_behavior/ssg-rhel7-oom-tailoring.xml diff --git a/tests/memory/ssg-rhel8-ds.xml.bz2 b/tests/probe_behavior/ssg-rhel8-ds.xml.bz2 similarity index 100% rename from tests/memory/ssg-rhel8-ds.xml.bz2 rename to tests/probe_behavior/ssg-rhel8-ds.xml.bz2 diff --git a/tests/memory/ssg-rhel8-oom-tailoring.xml b/tests/probe_behavior/ssg-rhel8-oom-tailoring.xml similarity index 100% rename from tests/memory/ssg-rhel8-oom-tailoring.xml rename to tests/probe_behavior/ssg-rhel8-oom-tailoring.xml