Remove multiple calls to free when successively calling jq_reset. (#3134)
`jq_reset` calls `jv_free` on the `exit_code` and the `error_message` stored on the jq state. However, it doesn't replace the actual instance of those members. This means that subsequent calls to `jq_reset` will call `jv_free` again on those members, which in turn may call `free` on the same pointer multiple times. Freeing the same pointer multiple times is undefined behavior and can cause heap corruption, which is how I spotted this issue. In practice, this issue only occurs when using a program that may `halt_error`, because that is when the `exit_code` and `error_message` are set to values other than `jv_invalid`. Subsequent attempts to call `jq_start` (which calls `jq_reset` internally) after hitting a `halt_error` can cause you to run into this issue. The changes simply reset the `exit_code` and the `error_message` to `jv_invalid` (the initial value set in `jq_init`) after they are freed.
This commit is contained in:
1 parent
5208a4425f
commit
7be6870751
2 files changed
+64
No files matched your search
@@ -315,7 +315,9 @@ static void jq_reset(jq_state *jq) {
|
||||
|
||||
jq->halted = 0;
|
||||
jv_free(jq->exit_code);
|
||||
jq->exit_code = jv_invalid();
|
||||
jv_free(jq->error_message);
|
||||
jq->error_message = jv_invalid();
|
||||
if (jv_get_kind(jq->path) != JV_KIND_INVALID)
|
||||
jv_free(jq->path);
|
||||
jq->path = jv_null();
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
|
||||
static void jv_test();
|
||||
static void run_jq_tests(jv, int, FILE *, int, int);
|
||||
static void run_jq_start_state_tests();
|
||||
#ifdef HAVE_PTHREAD
|
||||
static void run_jq_pthread_tests();
|
||||
#endif
|
||||
@@ -37,6 +38,7 @@ int jq_testsuite(jv libdirs, int verbose, int argc, char* argv[]) {
|
||||
}
|
||||
}
|
||||
run_jq_tests(libdirs, verbose, testdata, skip, take);
|
||||
run_jq_start_state_tests();
|
||||
#ifdef HAVE_PTHREAD
|
||||
run_jq_pthread_tests();
|
||||
#endif
|
||||
@@ -251,6 +253,66 @@ static void run_jq_tests(jv lib_dirs, int verbose, FILE *testdata, int skip, int
|
||||
}
|
||||
|
||||
|
||||
static int test_start_state(jq_state *jq, char *prog) {
|
||||
int pass = 1;
|
||||
jv message = jq_get_error_message(jq);
|
||||
if (jv_is_valid(message)) {
|
||||
printf("*** Expected error_message to be invalid after jq_start: %s\n", prog);
|
||||
pass = 0;
|
||||
}
|
||||
jv_free(message);
|
||||
|
||||
jv exit_code = jq_get_exit_code(jq);
|
||||
if (jv_is_valid(exit_code)) {
|
||||
printf("*** Expected exit_code to be invalid after jq_start: %s\n", prog);
|
||||
pass = 0;
|
||||
}
|
||||
jv_free(exit_code);
|
||||
|
||||
if (jq_halted(jq)) {
|
||||
printf("*** Expected jq to not be halted after jq_start: %s\n", prog);
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
return pass;
|
||||
}
|
||||
|
||||
// Test jq_state is reset after subsequent calls to jq_start.
|
||||
static void test_jq_start_resets_state(char *prog, const char *input) {
|
||||
printf("Test jq_state: %s\n", prog);
|
||||
jq_state *jq = jq_init();
|
||||
assert(jq);
|
||||
|
||||
int compiled = jq_compile(jq, prog);
|
||||
assert(compiled);
|
||||
|
||||
// First call to jq_start. Run until completion.
|
||||
jv parsed_input = jv_parse(input);
|
||||
assert(jv_is_valid(parsed_input));
|
||||
jq_start(jq, parsed_input, 0);
|
||||
assert(test_start_state(jq, prog));
|
||||
while (1) {
|
||||
jv result = jq_next(jq);
|
||||
int valid = jv_is_valid(result);
|
||||
jv_free(result);
|
||||
if (!valid) { break; }
|
||||
}
|
||||
|
||||
// Second call to jq_start.
|
||||
jv parsed_input2 = jv_parse(input);
|
||||
assert(jv_is_valid(parsed_input2));
|
||||
jq_start(jq, parsed_input2, 0);
|
||||
assert(test_start_state(jq, prog));
|
||||
|
||||
jq_teardown(&jq);
|
||||
}
|
||||
|
||||
static void run_jq_start_state_tests() {
|
||||
test_jq_start_resets_state(".[]", "[1,2,3]");
|
||||
test_jq_start_resets_state(".[] | if .%2 == 0 then halt_error else . end", "[1,2,3]");
|
||||
}
|
||||
|
||||
|
||||
/// pthread regression test
|
||||
#ifdef HAVE_PTHREAD
|
||||
#define NUMBER_OF_THREADS 3
|
||||
|
||||
Reference in new issue
Block a user