aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorRobin Haberkorn <robin.haberkorn@googlemail.com>2021-06-04 01:17:38 +0200
committerRobin Haberkorn <robin.haberkorn@googlemail.com>2021-06-04 01:30:18 +0200
commit5167dad198508e2dac10bf89c6b2991cfc791ee6 (patch)
tree7dbab3e7047ce80e6651f238f95d241a6a460ad1
parentad392b343a59c6bd093a2bd352598f6e7747828e (diff)
downloadsciteco-5167dad198508e2dac10bf89c6b2991cfc791ee6.tar.gz
guard against too low arguments to <S> by checking whether the memory limit would be exceeded
* Checking whether the allocation succeeded may not prevent exceeding the memory limit excessively. * Even if the memory limit is not exceeded, the allocation can fail theoretically and the program would terminate abnormally. This however is true for all allocations in SciTECO (via glib). * teco_memory_check() therefore now supports checking whether an allocation would exceed the memory limit which will be useful before very large or variable allocations in addition to the regular checking in teco_machine_main_step(). * As a sideeffect, this fixes the "Searching with large counts" test case on Mac OS where too large allocations were not detected as expected (apparently Mac OS happily gives out ridiculously large chunks of memory). Now, all platforms are guaranteed to have the same behaviour.
-rw-r--r--src/memory.c11
-rw-r--r--src/memory.h2
-rw-r--r--src/parser.c6
-rw-r--r--src/search.c18
-rw-r--r--src/spawn.c5
-rw-r--r--tests/testsuite.at2
6 files changed, 29 insertions, 15 deletions
diff --git a/src/memory.c b/src/memory.c
index c80ccc7..302939b 100644
--- a/src/memory.c
+++ b/src/memory.c
@@ -658,10 +658,17 @@ teco_memory_set_limit(gsize new_limit, GError **error)
return TRUE;
}
+/**
+ * Check whether the memory limit is exceeded or would be
+ * exceeded by an allocation.
+ *
+ * @param request Size of the requested allocation or 0 if
+ * you want to check the current memory usage.
+ */
gboolean
-teco_memory_check(GError **error)
+teco_memory_check(gsize request, GError **error)
{
- gsize memory_usage = g_atomic_int_get(&teco_memory_usage);
+ gsize memory_usage = g_atomic_int_get(&teco_memory_usage) + request;
if (G_UNLIKELY(teco_memory_limit && memory_usage > teco_memory_limit)) {
g_autofree gchar *limit_str = g_format_size(memory_usage);
diff --git a/src/memory.h b/src/memory.h
index 58705a7..b2160e7 100644
--- a/src/memory.h
+++ b/src/memory.h
@@ -25,4 +25,4 @@ void teco_memory_stop_limiting(void);
gboolean teco_memory_set_limit(gsize new_limit, GError **error);
-gboolean teco_memory_check(GError **error);
+gboolean teco_memory_check(gsize request, GError **error);
diff --git a/src/parser.c b/src/parser.c
index ff1fd18..22f7a4f 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -105,7 +105,11 @@ teco_machine_main_step(teco_machine_main_t *ctx, const gchar *macro, gint stop_p
goto error_attach;
}
- if (!teco_memory_check(error))
+ /*
+ * Most allocations are small or of limited size,
+ * so it is (almost) sufficient to check the memory limit regularily.
+ */
+ if (!teco_memory_check(0, error))
goto error_attach;
if (!teco_machine_input(&ctx->parent, macro[ctx->macro_pc], error))
diff --git a/src/search.c b/src/search.c
index e5e4bd8..4c324a6 100644
--- a/src/search.c
+++ b/src/search.c
@@ -28,6 +28,7 @@
#include "string-utils.h"
#include "expressions.h"
#include "interface.h"
+#include "memory.h"
#include "undo.h"
#include "qreg.h"
#include "ring.h"
@@ -492,17 +493,18 @@ teco_do_search(GRegex *re, gint from, gint to, gint *count, GError **error)
gint from, to;
} teco_range_t;
+ gsize matched_size = sizeof(teco_range_t) * -*count;
+
/*
- * NOTE: It's theoretically possible that this single allocation
- * causes an OOM if (-count) is large enough and memory limiting won't help.
- * That's why we exceptionally have to check for allocation success.
+ * NOTE: It's theoretically possible that the allocation of the `matched`
+ * array causes an OOM if (-count) is large enough and regular
+ * memory limiting in teco_machine_main_step() wouldn't help.
+ * That's why we exceptionally have to check before allocating.
*/
- g_autofree teco_range_t *matched = g_try_new(teco_range_t, -*count);
- if (!matched) {
- g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED,
- "Search count too small (%d)", *count);
+ if (!teco_memory_check(matched_size, error))
return FALSE;
- }
+
+ g_autofree teco_range_t *matched = g_malloc(matched_size);
gint matched_total = 0, i = 0;
diff --git a/src/spawn.c b/src/spawn.c
index 1406731..00ff3ad 100644
--- a/src/spawn.c
+++ b/src/spawn.c
@@ -640,10 +640,11 @@ teco_spawn_stdout_watch_cb(GIOChannel *chan, GIOCondition condition, gpointer da
teco_spawn_ctx.text_added = TRUE;
/*
- * NOTE: Since this reads from an external process, we could insert
+ * NOTE: Since this reads from an external process and regular memory
+ * limiting in teco_machine_main_step() is not performed, we could insert
* indefinitely (eg. cat /dev/zero).
*/
- if (!teco_memory_check(&teco_spawn_ctx.error))
+ if (!teco_memory_check(0, &teco_spawn_ctx.error))
goto error;
}
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 6c592a0..beed747 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -65,7 +65,7 @@ AT_SETUP([Searching with large counts])
# Even though the search will be unsuccessful, it will not be considered
# a proper error, so the process return code is still 0.
AT_CHECK([$SCITECO -e "2147483647@S/foo/"], 0, ignore, ignore)
-# NOTE: In case of crashes, the return code should be >= 128 (at least on Linux).
+# Will always break the memory limit which is considered an error.
AT_CHECK([$SCITECO -e "-2147483648@S/foo/"], 1, ignore, ignore)
AT_CLEANUP