From 9674b089cfb1f75653579e83735e049ddcbbed7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 10:24:36 +0200 Subject: [PATCH 01/11] json: align table --- src/shared/json.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index f66b7df24c..9e44e4b68e 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4754,7 +4754,6 @@ bool json_variant_is_sorted(JsonVariant *v) { } int json_variant_unbase64(JsonVariant *v, void **ret, size_t *ret_size) { - if (!json_variant_is_string(v)) return -EINVAL; @@ -4762,7 +4761,6 @@ int json_variant_unbase64(JsonVariant *v, void **ret, size_t *ret_size) { } int json_variant_unhex(JsonVariant *v, void **ret, size_t *ret_size) { - if (!json_variant_is_string(v)) return -EINVAL; @@ -4770,15 +4768,15 @@ int json_variant_unhex(JsonVariant *v, void **ret, size_t *ret_size) { } static const char* const json_variant_type_table[_JSON_VARIANT_TYPE_MAX] = { - [JSON_VARIANT_STRING] = "string", - [JSON_VARIANT_INTEGER] = "integer", + [JSON_VARIANT_STRING] = "string", + [JSON_VARIANT_INTEGER] = "integer", [JSON_VARIANT_UNSIGNED] = "unsigned", - [JSON_VARIANT_REAL] = "real", - [JSON_VARIANT_NUMBER] = "number", - [JSON_VARIANT_BOOLEAN] = "boolean", - [JSON_VARIANT_ARRAY] = "array", - [JSON_VARIANT_OBJECT] = "object", - [JSON_VARIANT_NULL] = "null", + [JSON_VARIANT_REAL] = "real", + [JSON_VARIANT_NUMBER] = "number", + [JSON_VARIANT_BOOLEAN] = "boolean", + [JSON_VARIANT_ARRAY] = "array", + [JSON_VARIANT_OBJECT] = "object", + [JSON_VARIANT_NULL] = "null", }; DEFINE_STRING_TABLE_LOOKUP(json_variant_type, JsonVariantType); From 9ad955ce40e244a52984c68ae2a6b151d918b4a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 10:41:36 +0200 Subject: [PATCH 02/11] fuzz-json: optionally allow logging and output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similarly to other fuzzers… this makes development easier. --- src/fuzz/fuzz-json.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/fuzz/fuzz-json.c b/src/fuzz/fuzz-json.c index ad7460c6fd..648a6d441d 100644 --- a/src/fuzz/fuzz-json.c +++ b/src/fuzz/fuzz-json.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "alloc-util.h" +#include "env-util.h" #include "fd-util.h" #include "fuzz.h" #include "json.h" @@ -10,18 +11,26 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { size_t out_size; _cleanup_fclose_ FILE *f = NULL, *g = NULL; _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; + int r; + + /* Disable most logging if not running standalone */ + if (!getenv("SYSTEMD_LOG_LEVEL")) + log_set_max_level(LOG_CRIT); f = data_to_file(data, size); assert_se(f); - if (json_parse_file(f, NULL, 0, &v, NULL, NULL) < 0) + r = json_parse_file(f, NULL, 0, &v, NULL, NULL); + if (r < 0) { + log_debug_errno(r, "failed to parse input: %m"); return 0; + } - g = open_memstream_unlocked(&out, &out_size); - assert_se(g); + if (getenv_bool("SYSTEMD_FUZZ_OUTPUT") <= 0) + assert_se(g = open_memstream_unlocked(&out, &out_size)); - json_variant_dump(v, 0, g, NULL); - json_variant_dump(v, JSON_FORMAT_PRETTY|JSON_FORMAT_COLOR|JSON_FORMAT_SOURCE, g, NULL); + json_variant_dump(v, 0, g ?: stdout, NULL); + json_variant_dump(v, JSON_FORMAT_PRETTY|JSON_FORMAT_COLOR|JSON_FORMAT_SOURCE, g ?: stdout, NULL); return 0; } From a4669764f7329d1e8a3d364db519500355cea5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 11:01:32 +0200 Subject: [PATCH 03/11] shared/json: reduce scope of variables --- src/shared/json.c | 131 +++++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 78 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index 9e44e4b68e..df80294c0d 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -574,9 +574,6 @@ int json_variant_new_array(JsonVariant **ret, JsonVariant **array, size_t n) { } int json_variant_new_array_bytes(JsonVariant **ret, const void *p, size_t n) { - JsonVariant *v; - size_t i; - assert_return(ret, -EINVAL); if (n == 0) { *ret = JSON_VARIANT_MAGIC_EMPTY_ARRAY; @@ -584,7 +581,7 @@ int json_variant_new_array_bytes(JsonVariant **ret, const void *p, size_t n) { } assert_return(p, -EINVAL); - v = new(JsonVariant, n + 1); + JsonVariant *v = new(JsonVariant, n + 1); if (!v) return -ENOMEM; @@ -595,7 +592,7 @@ int json_variant_new_array_bytes(JsonVariant **ret, const void *p, size_t n) { .depth = 1, }; - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { JsonVariant *w = v + 1 + i; *w = (JsonVariant) { @@ -790,12 +787,9 @@ static void json_variant_free_inner(JsonVariant *v, bool force_sensitive) { return; } - if (IN_SET(v->type, JSON_VARIANT_ARRAY, JSON_VARIANT_OBJECT)) { - size_t i; - - for (i = 0; i < v->n_elements; i++) + if (IN_SET(v->type, JSON_VARIANT_ARRAY, JSON_VARIANT_OBJECT)) + for (size_t i = 0; i < v->n_elements; i++) json_variant_free_inner(v + 1 + i, sensitive); - } if (sensitive) explicit_bzero_safe(v, json_variant_size(v)); @@ -839,11 +833,9 @@ JsonVariant *json_variant_unref(JsonVariant *v) { } void json_variant_unref_many(JsonVariant **array, size_t n) { - size_t i; - assert(array || n == 0); - for (i = 0; i < n; i++) + for (size_t i = 0; i < n; i++) json_variant_unref(array[i]); } @@ -1218,8 +1210,6 @@ mismatch: } JsonVariant *json_variant_by_key_full(JsonVariant *v, const char *key, JsonVariant **ret_key) { - size_t i; - if (!v) goto not_found; if (!key) @@ -1241,6 +1231,7 @@ JsonVariant *json_variant_by_key_full(JsonVariant *v, const char *key, JsonVaria while (b > a) { JsonVariant *p; const char *f; + size_t i; int c; i = (a + b) / 2; @@ -1264,7 +1255,7 @@ JsonVariant *json_variant_by_key_full(JsonVariant *v, const char *key, JsonVaria } /* The variant is not sorted, hence search for the field linearly */ - for (i = 0; i < v->n_elements; i += 2) { + for (size_t i = 0; i < v->n_elements; i += 2) { JsonVariant *p; p = json_variant_dereference(v + 1 + i); @@ -1335,34 +1326,28 @@ bool json_variant_equal(JsonVariant *a, JsonVariant *b) { return true; case JSON_VARIANT_ARRAY: { - size_t i, n; - - n = json_variant_elements(a); + size_t n = json_variant_elements(a); if (n != json_variant_elements(b)) return false; - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) if (!json_variant_equal(json_variant_by_index(a, i), json_variant_by_index(b, i))) return false; - } return true; } case JSON_VARIANT_OBJECT: { - size_t i, n; - - n = json_variant_elements(a); + size_t n = json_variant_elements(a); if (n != json_variant_elements(b)) return false; /* Iterate through all keys in 'a' */ - for (i = 0; i < n; i += 2) { + for (size_t i = 0; i < n; i += 2) { bool found = false; - size_t j; /* Match them against all keys in 'b' */ - for (j = 0; j < n; j += 2) { + for (size_t j = 0; j < n; j += 2) { JsonVariant *key_b; key_b = json_variant_by_index(b, j); @@ -1470,16 +1455,14 @@ static int print_source(FILE *f, JsonVariant *v, JsonFormatFlags flags, bool whi DECIMAL_STR_MAX(unsigned) -1; if (whitespace) { - size_t i, n; + size_t n = 1 + (v->source ? strlen(v->source->name) : 0) + + ((v->source && (v->line > 0 || v->column > 0)) ? 1 : 0) + + (v->line > 0 ? w : 0) + + (((v->source || v->line > 0) && v->column > 0) ? 1 : 0) + + (v->column > 0 ? k : 0) + + 2; - n = 1 + (v->source ? strlen(v->source->name) : 0) + - ((v->source && (v->line > 0 || v->column > 0)) ? 1 : 0) + - (v->line > 0 ? w : 0) + - (((v->source || v->line > 0) && v->column > 0) ? 1 : 0) + - (v->column > 0 ? k : 0) + - 2; - - for (i = 0; i < n; i++) + for (size_t i = 0; i < n; i++) fputc(' ', f); } else { fputc('[', f); @@ -1631,10 +1614,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha break; case JSON_VARIANT_ARRAY: { - size_t i, n; - - n = json_variant_elements(v); - + size_t n = json_variant_elements(v); if (n == 0) fputs("[]", f); else { @@ -1653,7 +1633,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha fputc('[', f); } - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { JsonVariant *e; assert_se(e = json_variant_by_index(v, i)); @@ -1687,10 +1667,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha } case JSON_VARIANT_OBJECT: { - size_t i, n; - - n = json_variant_elements(v); - + size_t n = json_variant_elements(v); if (n == 0) fputs("{}", f); else { @@ -1709,7 +1686,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha fputc('{', f); } - for (i = 0; i < n; i += 2) { + for (size_t i = 0; i < n; i += 2) { JsonVariant *e; e = json_variant_by_index(v, i); @@ -1826,7 +1803,7 @@ void json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const cha int json_variant_filter(JsonVariant **v, char **to_remove) { _cleanup_(json_variant_unrefp) JsonVariant *w = NULL; _cleanup_free_ JsonVariant **array = NULL; - size_t i, n = 0, k = 0; + size_t n = 0, k = 0; int r; assert(v); @@ -1839,7 +1816,7 @@ int json_variant_filter(JsonVariant **v, char **to_remove) { if (strv_isempty(to_remove)) return 0; - for (i = 0; i < json_variant_elements(*v); i += 2) { + for (size_t i = 0; i < json_variant_elements(*v); i += 2) { JsonVariant *p; p = json_variant_by_index(*v, i); @@ -1881,7 +1858,7 @@ int json_variant_filter(JsonVariant **v, char **to_remove) { int json_variant_set_field(JsonVariant **v, const char *field, JsonVariant *value) { _cleanup_(json_variant_unrefp) JsonVariant *field_variant = NULL, *w = NULL; _cleanup_free_ JsonVariant **array = NULL; - size_t i, k = 0; + size_t k = 0; int r; assert(v); @@ -1896,7 +1873,7 @@ int json_variant_set_field(JsonVariant **v, const char *field, JsonVariant *valu if (!json_variant_is_object(*v)) return -EINVAL; - for (i = 0; i < json_variant_elements(*v); i += 2) { + for (size_t i = 0; i < json_variant_elements(*v); i += 2) { JsonVariant *p; p = json_variant_by_index(*v, i); @@ -2007,7 +1984,7 @@ int json_variant_set_field_strv(JsonVariant **v, const char *field, char **l) { int json_variant_merge(JsonVariant **v, JsonVariant *m) { _cleanup_(json_variant_unrefp) JsonVariant *w = NULL; _cleanup_free_ JsonVariant **array = NULL; - size_t v_elements, m_elements, i, k; + size_t v_elements, m_elements, k; bool v_blank, m_blank; int r; @@ -2040,7 +2017,7 @@ int json_variant_merge(JsonVariant **v, JsonVariant *m) { return -ENOMEM; k = 0; - for (i = 0; i < v_elements; i += 2) { + for (size_t i = 0; i < v_elements; i += 2) { JsonVariant *u; u = json_variant_by_index(*v, i); @@ -2054,7 +2031,7 @@ int json_variant_merge(JsonVariant **v, JsonVariant *m) { array[k++] = json_variant_by_index(*v, i + 1); } - for (i = 0; i < m_elements; i++) + for (size_t i = 0; i < m_elements; i++) array[k++] = json_variant_by_index(m, i); r = json_variant_new_object(&w, array, k); @@ -2089,19 +2066,17 @@ int json_variant_append_array(JsonVariant **v, JsonVariant *element) { if (blank) r = json_variant_new_array(&nv, (JsonVariant*[]) { element }, 1); else { - _cleanup_free_ JsonVariant **array = NULL; - size_t i; - - array = new(JsonVariant*, json_variant_elements(*v) + 1); + _cleanup_free_ JsonVariant **array = new(JsonVariant*, json_variant_elements(*v) + 1); if (!array) return -ENOMEM; - for (i = 0; i < json_variant_elements(*v); i++) + size_t size = json_variant_elements(*v); + for (size_t i = 0; i < size; i++) array[i] = json_variant_by_index(*v, i); - array[i] = element; + array[size] = element; - r = json_variant_new_array(&nv, array, i + 1); + r = json_variant_new_array(&nv, array, size + 1); } if (r < 0) return r; @@ -2116,7 +2091,6 @@ int json_variant_append_array(JsonVariant **v, JsonVariant *element) { int json_variant_strv(JsonVariant *v, char ***ret) { char **l = NULL; - size_t n, i; bool sensitive; int r; @@ -2136,12 +2110,12 @@ int json_variant_strv(JsonVariant *v, char ***ret) { sensitive = v->sensitive; - n = json_variant_elements(v); + size_t n = json_variant_elements(v); l = new(char*, n+1); if (!l) return -ENOMEM; - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { JsonVariant *e; assert_se(e = json_variant_by_index(v, i)); @@ -2160,7 +2134,7 @@ int json_variant_strv(JsonVariant *v, char ***ret) { } } - l[i] = NULL; + l[n] = NULL; *ret = TAKE_PTR(l); return 0; @@ -2847,7 +2821,7 @@ static int json_parse_internal( unsigned *column, bool continue_end) { - size_t n_stack = 1, i; + size_t n_stack = 1; unsigned line_buffer = 0, column_buffer = 0; void *tokenizer_state = NULL; JsonStack *stack = NULL; @@ -3186,7 +3160,7 @@ done: r = 0; finish: - for (i = 0; i < n_stack; i++) + for (size_t i = 0; i < n_stack; i++) json_stack_release(stack + i); free(stack); @@ -3229,7 +3203,7 @@ int json_parse_file_at(FILE *f, int dir_fd, const char *path, JsonParseFlags fla int json_buildv(JsonVariant **ret, va_list ap) { JsonStack *stack = NULL; - size_t n_stack = 1, i; + size_t n_stack = 1; int r; assert_return(ret, -EINVAL); @@ -4147,7 +4121,7 @@ done: r = 0; finish: - for (i = 0; i < n_stack; i++) + for (size_t i = 0; i < n_stack; i++) json_stack_release(stack + i); free(stack); @@ -4231,8 +4205,7 @@ int json_log_internal( } int json_dispatch(JsonVariant *v, const JsonDispatch table[], JsonDispatchCallback bad, JsonDispatchFlags flags, void *userdata) { - const JsonDispatch *p; - size_t i, n, m; + size_t m; int r, done = 0; bool *found; @@ -4245,14 +4218,16 @@ int json_dispatch(JsonVariant *v, const JsonDispatch table[], JsonDispatchCallba return -EINVAL; } - for (p = table, m = 0; p->name; p++) + m = 0; + for (const JsonDispatch *p = table; p->name; p++) m++; found = newa0(bool, m); - n = json_variant_elements(v); - for (i = 0; i < n; i += 2) { + size_t n = json_variant_elements(v); + for (size_t i = 0; i < n; i += 2) { JsonVariant *key, *value; + const JsonDispatch *p; assert_se(key = json_variant_by_index(v, i)); assert_se(value = json_variant_by_index(v, i+1)); @@ -4326,7 +4301,7 @@ int json_dispatch(JsonVariant *v, const JsonDispatch table[], JsonDispatchCallba } } - for (p = table; p->name; p++) { + for (const JsonDispatch *p = table; p->name; p++) { JsonDispatchFlags merged_flags = p->flags | flags; if ((merged_flags & JSON_MANDATORY) && !found[p-table]) { @@ -4621,7 +4596,7 @@ static int json_cmp_strings(const void *x, const void *y) { int json_variant_sort(JsonVariant **v) { _cleanup_free_ JsonVariant **a = NULL; JsonVariant *n = NULL; - size_t i, m; + size_t m; int r; assert(v); @@ -4639,7 +4614,7 @@ int json_variant_sort(JsonVariant **v) { if (!a) return -ENOMEM; - for (i = 0; i < m; i++) + for (size_t i = 0; i < m; i++) a[i] = json_variant_by_index(*v, i); qsort(a, m/2, sizeof(JsonVariant*)*2, json_cmp_strings); @@ -4662,7 +4637,7 @@ int json_variant_sort(JsonVariant **v) { int json_variant_normalize(JsonVariant **v) { _cleanup_free_ JsonVariant **a = NULL; JsonVariant *n = NULL; - size_t i, j, m; + size_t i, m; int r; assert(v); @@ -4714,7 +4689,7 @@ int json_variant_normalize(JsonVariant **v) { r = 1; finish: - for (j = 0; j < i; j++) + for (size_t j = 0; j < i; j++) json_variant_unref(a[j]); return r; From a2c5735dd80e19d2d525f9be322395530096cbe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 11:07:06 +0200 Subject: [PATCH 04/11] fuzz-json: also do sorting and normalizing and other easy calls --- src/fuzz/fuzz-json.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/fuzz/fuzz-json.c b/src/fuzz/fuzz-json.c index 648a6d441d..995a0265ba 100644 --- a/src/fuzz/fuzz-json.c +++ b/src/fuzz/fuzz-json.c @@ -32,5 +32,80 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { json_variant_dump(v, 0, g ?: stdout, NULL); json_variant_dump(v, JSON_FORMAT_PRETTY|JSON_FORMAT_COLOR|JSON_FORMAT_SOURCE, g ?: stdout, NULL); + bool sorted = json_variant_is_sorted(v); + log_debug("json_variant_is_sorted: %s", yes_no(sorted)); + + r = json_variant_sort(&v); + log_debug_errno(r, "json_variant_sort: %d/%m", r); + + sorted = json_variant_is_sorted(v); + log_debug("json_variant_is_sorted: %s", yes_no(sorted)); + assert_se(r < 0 || sorted); + + bool normalized = json_variant_is_normalized(v); + log_debug("json_variant_is_normalized: %s", yes_no(normalized)); + + r = json_variant_normalize(&v); + log_debug_errno(r, "json_variant_normalize: %d/%m", r); + + normalized = json_variant_is_normalized(v); + log_debug("json_variant_is_normalized: %s", yes_no(normalized)); + assert_se(r < 0 || normalized); + + double real = json_variant_real(v); + log_debug("json_variant_real: %lf", real); + + bool negative = json_variant_is_negative(v); + log_debug("json_variant_is_negative: %s", yes_no(negative)); + + bool blank = json_variant_is_blank_object(v); + log_debug("json_variant_is_blank_object: %s", yes_no(blank)); + + blank = json_variant_is_blank_array(v); + log_debug("json_variant_is_blank_array: %s", yes_no(blank)); + + size_t elements = json_variant_elements(v); + log_debug("json_variant_elements: %zu", elements); + + for (size_t i = 0; i <= elements + 2; i++) + (void) json_variant_by_index(v, i); + + assert_se(json_variant_equal(v, v)); + assert_se(!json_variant_equal(v, NULL)); + assert_se(!json_variant_equal(NULL, v)); + + bool sensitive = json_variant_is_sensitive(v); + log_debug("json_variant_is_sensitive: %s", yes_no(sensitive)); + + json_variant_sensitive(v); + + sensitive = json_variant_is_sensitive(v); + log_debug("json_variant_is_sensitive: %s", yes_no(sensitive)); + + const char *source; + unsigned line, column; + assert_se(json_variant_get_source(v, &source, &line, &column) == 0); + log_debug("json_variant_get_source: %s:%u:%u", source ?: "-", line, column); + + r = json_variant_set_field_string(&v, "a", "string-a"); + log_debug_errno(r, "json_set_field_string: %d/%m", r); + + r = json_variant_set_field_integer(&v, "b", -12345); + log_debug_errno(r, "json_set_field_integer: %d/%m", r); + + r = json_variant_set_field_unsigned(&v, "c", 12345); + log_debug_errno(r, "json_set_field_unsigned: %d/%m", r); + + r = json_variant_set_field_boolean(&v, "d", false); + log_debug_errno(r, "json_set_field_boolean: %d/%m", r); + + r = json_variant_set_field_strv(&v, "e", STRV_MAKE("e-1", "e-2", "e-3")); + log_debug_errno(r, "json_set_field_strv: %d/%m", r); + + r = json_variant_filter(&v, STRV_MAKE("a", "b", "c", "d", "e")); + log_debug_errno(r, "json_variant_filter: %d/%m", r); + + // TODO: json_variant_merge(&v, …); + // TODO: json_variant_append_array(&v, …); return 0; } From bac06497feda9eb8c485243f2e05a7f7c112616c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 13:37:35 +0200 Subject: [PATCH 05/11] shared/json: wrap long comments --- src/shared/json.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index df80294c0d..55e6d95cdb 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -273,8 +273,8 @@ static JsonVariant *json_variant_formalize(JsonVariant *v) { static JsonVariant *json_variant_conservative_formalize(JsonVariant *v) { - /* Much like json_variant_formalize(), but won't simplify if the variant has a source/line location attached to - * it, in order not to lose context */ + /* Much like json_variant_formalize(), but won't simplify if the variant has a source/line location + * attached to it, in order not to lose context */ if (!v) return NULL; @@ -546,7 +546,7 @@ int json_variant_new_array(JsonVariant **ret, JsonVariant **array, size_t n) { for (v->n_elements = 0; v->n_elements < n; v->n_elements++) { JsonVariant *w = v + 1 + v->n_elements, - *c = array[v->n_elements]; + *c = array[v->n_elements]; uint16_t d; d = json_variant_depth(c); @@ -690,7 +690,7 @@ int json_variant_new_object(JsonVariant **ret, JsonVariant **array, size_t n) { for (v->n_elements = 0; v->n_elements < n; v->n_elements++) { JsonVariant *w = v + 1 + v->n_elements, - *c = array[v->n_elements]; + *c = array[v->n_elements]; uint16_t d; if ((v->n_elements & 1) == 0) { @@ -731,7 +731,6 @@ int json_variant_new_object(JsonVariant **ret, JsonVariant **array, size_t n) { } static size_t json_variant_size(JsonVariant* v) { - if (!json_variant_is_regular(v)) return 0; @@ -2253,8 +2252,9 @@ static int json_variant_set_source(JsonVariant **v, JsonSource *source, unsigned assert(v); - /* Patch in source and line/column number. Tries to do this in-place if the caller is the sole referencer of - * the object. If not, allocates a new object, possibly a surrogate for the original one */ + /* Patch in source and line/column number. Tries to do this in-place if the caller is the sole + * referencer of the object. If not, allocates a new object, possibly a surrogate for the original + * one */ if (!*v) return 0; @@ -3731,10 +3731,10 @@ int json_buildv(JsonVariant **ret, va_list ap) { stack[n_stack++] = (JsonStack) { .expect = EXPECT_OBJECT_KEY, - .n_suppress = current->n_suppress != 0 ? SIZE_MAX : 0, /* if we shall suppress the - * new object, then we should - * also suppress all object - * members */ + .n_suppress = current->n_suppress != 0 ? SIZE_MAX : 0, /* If we shall suppress the + * new object, then we should + * also suppress all object + * members. */ }; break; @@ -4102,9 +4102,9 @@ int json_buildv(JsonVariant **ret, va_list ap) { current->elements[current->n_elements++] = TAKE_PTR(add_more); } - /* If we are supposed to suppress items, let's subtract how many items where generated from that - * counter. Except if the counter is SIZE_MAX, i.e. we shall suppress an infinite number of elements - * on this stack level */ + /* If we are supposed to suppress items, let's subtract how many items where generated from + * that counter. Except if the counter is SIZE_MAX, i.e. we shall suppress an infinite number + * of elements on this stack level */ if (current->n_suppress != SIZE_MAX) { if (current->n_suppress <= n_subtract) /* Saturated */ current->n_suppress = 0; @@ -4696,10 +4696,9 @@ finish: } bool json_variant_is_normalized(JsonVariant *v) { - - /* For now, let's consider anything containing numbers not expressible as integers as - * non-normalized. That's because we cannot sensibly compare them due to accuracy issues, nor even - * store them if they are too large. */ + /* For now, let's consider anything containing numbers not expressible as integers as non-normalized. + * That's because we cannot sensibly compare them due to accuracy issues, nor even store them if they + * are too large. */ if (json_variant_is_real(v) && !json_variant_is_integer(v) && !json_variant_is_unsigned(v)) return false; From 7e4be6a5845f983a299932d4ccb2c4349cf8dd52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 14:28:36 +0200 Subject: [PATCH 06/11] shared/json: fix memory leak on failed normalization We need to increase the counter immediately after taking the ref, otherwise we may not unref it properly if we fail before incrementing. --- src/shared/json.c | 5 +++-- test/fuzz/fuzz-json/leak-normalize-fail | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 test/fuzz/fuzz-json/leak-normalize-fail diff --git a/src/shared/json.c b/src/shared/json.c index 55e6d95cdb..2e52443dde 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4655,10 +4655,11 @@ int json_variant_normalize(JsonVariant **v) { if (!a) return -ENOMEM; - for (i = 0; i < m; i++) { + for (i = 0; i < m; ) { a[i] = json_variant_ref(json_variant_by_index(*v, i)); + i++; - r = json_variant_normalize(a + i); + r = json_variant_normalize(&a[i-1]); if (r < 0) goto finish; } diff --git a/test/fuzz/fuzz-json/leak-normalize-fail b/test/fuzz/fuzz-json/leak-normalize-fail new file mode 100644 index 0000000000..b247ccd199 --- /dev/null +++ b/test/fuzz/fuzz-json/leak-normalize-fail @@ -0,0 +1 @@ +[7E73] \ No newline at end of file From ce913e0ec4c97651c7c1509b72fb81ee61d80c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 15:10:36 +0200 Subject: [PATCH 07/11] shared/json: add helper to ref first, unref second This normally wouldn't happen, but if some of those places were called with lhs and rhs being the same object, we could unref the last ref first, and then try to take the ref again. It's easier to be safe, and with the helper we save some lines too. --- src/shared/json.c | 36 ++++++++++-------------------------- src/shared/json.h | 8 ++++++++ 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index 2e52443dde..b7978c3556 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -1847,9 +1847,7 @@ int json_variant_filter(JsonVariant **v, char **to_remove) { return r; json_variant_propagate_sensitive(*v, w); - - json_variant_unref(*v); - *v = TAKE_PTR(w); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(w)); return (int) n; } @@ -1918,9 +1916,7 @@ int json_variant_set_field(JsonVariant **v, const char *field, JsonVariant *valu return r; json_variant_propagate_sensitive(*v, w); - - json_variant_unref(*v); - *v = TAKE_PTR(w); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(w)); return 1; } @@ -2001,8 +1997,7 @@ int json_variant_merge(JsonVariant **v, JsonVariant *m) { return 0; /* nothing to do */ if (v_blank) { - json_variant_unref(*v); - *v = json_variant_ref(m); + JSON_VARIANT_REPLACE(*v, json_variant_ref(m)); return 1; } @@ -2039,9 +2034,7 @@ int json_variant_merge(JsonVariant **v, JsonVariant *m) { json_variant_propagate_sensitive(*v, w); json_variant_propagate_sensitive(m, w); - - json_variant_unref(*v); - *v = TAKE_PTR(w); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(w)); return 1; } @@ -2081,9 +2074,7 @@ int json_variant_append_array(JsonVariant **v, JsonVariant *element) { return r; json_variant_propagate_sensitive(*v, nv); - - json_variant_unref(*v); - *v = TAKE_PTR(nv); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(nv)); return 0; } @@ -2297,8 +2288,7 @@ static int json_variant_set_source(JsonVariant **v, JsonSource *source, unsigned w->line = line; w->column = column; - json_variant_unref(*v); - *v = w; + JSON_VARIANT_REPLACE(*v, w); return 1; } @@ -4499,14 +4489,10 @@ int json_dispatch_strv(const char *name, JsonVariant *variant, JsonDispatchFlags } int json_dispatch_variant(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata) { - JsonVariant **p = userdata; - + JsonVariant **p = ASSERT_PTR(userdata); assert(variant); - assert(p); - - json_variant_unref(*p); - *p = json_variant_ref(variant); + JSON_VARIANT_REPLACE(*p, json_variant_ref(variant)); return 0; } @@ -4628,8 +4614,7 @@ int json_variant_sort(JsonVariant **v) { if (!n->sorted) /* Check if this worked. This will fail if there are multiple identical keys used. */ return -ENOTUNIQ; - json_variant_unref(*v); - *v = n; + JSON_VARIANT_REPLACE(*v, n); return 1; } @@ -4684,8 +4669,7 @@ int json_variant_normalize(JsonVariant **v) { goto finish; } - json_variant_unref(*v); - *v = n; + JSON_VARIANT_REPLACE(*v, n); r = 1; diff --git a/src/shared/json.h b/src/shared/json.h index 91d02a911e..98d184c309 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -82,6 +82,14 @@ JsonVariant *json_variant_ref(JsonVariant *v); JsonVariant *json_variant_unref(JsonVariant *v); void json_variant_unref_many(JsonVariant **array, size_t n); +#define JSON_VARIANT_REPLACE(v, q) \ + do { \ + typeof(v)* _v = &(v); \ + typeof(q) _q = (q); \ + json_variant_unref(*_v); \ + *_v = _q; \ + } while(0) + DEFINE_TRIVIAL_CLEANUP_FUNC(JsonVariant *, json_variant_unref); const char *json_variant_string(JsonVariant *v); From 96d651a22bf62e63080e489cb45e82bead11aa5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 15:11:20 +0200 Subject: [PATCH 08/11] basic/alloc-util: remove unnecessary parens Those symbols are not macros anymore, so we can drop parens. --- src/basic/alloc-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index 9dde770dab..155fbf5f1f 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -55,8 +55,8 @@ typedef void* (*mfree_func_t)(void *p); typeof(a)* _a = &(a); \ typeof(b)* _b = &(b); \ free(*_a); \ - (*_a) = (*_b); \ - (*_b) = NULL; \ + *_a = *_b; \ + *_b = NULL; \ 0; \ }) From dbd27c6d2830aeb7173933d1f4a9a07457e4092d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 May 2022 15:14:33 +0200 Subject: [PATCH 09/11] fuzz-json: also try self-merge operations This might even work ;) --- src/fuzz/fuzz-json.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/fuzz/fuzz-json.c b/src/fuzz/fuzz-json.c index 995a0265ba..c393fcf394 100644 --- a/src/fuzz/fuzz-json.c +++ b/src/fuzz/fuzz-json.c @@ -105,7 +105,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { r = json_variant_filter(&v, STRV_MAKE("a", "b", "c", "d", "e")); log_debug_errno(r, "json_variant_filter: %d/%m", r); - // TODO: json_variant_merge(&v, …); - // TODO: json_variant_append_array(&v, …); + /* I assume we can merge v with itself… */ + r = json_variant_merge(&v, v); + log_debug_errno(r, "json_variant_merge: %d/%m", r); + + r = json_variant_append_array(&v, v); + log_debug_errno(r, "json_variant_append_array: %d/%m", r); + return 0; } From 3b6ce05537cd3544a15073f920347cabd7a39450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 May 2022 09:05:43 +0200 Subject: [PATCH 10/11] shared/json: fix another memleak in normalization --- src/shared/json.c | 4 ++-- test/fuzz/fuzz-json/leak-normalize-object | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 test/fuzz/fuzz-json/leak-normalize-object diff --git a/src/shared/json.c b/src/shared/json.c index b7978c3556..d9eda88b12 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4621,7 +4621,7 @@ int json_variant_sort(JsonVariant **v) { int json_variant_normalize(JsonVariant **v) { _cleanup_free_ JsonVariant **a = NULL; - JsonVariant *n = NULL; + _cleanup_(json_variant_unrefp) JsonVariant *n = NULL; size_t i, m; int r; @@ -4669,7 +4669,7 @@ int json_variant_normalize(JsonVariant **v) { goto finish; } - JSON_VARIANT_REPLACE(*v, n); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(n)); r = 1; diff --git a/test/fuzz/fuzz-json/leak-normalize-object b/test/fuzz/fuzz-json/leak-normalize-object new file mode 100644 index 0000000000..0a8caa426c --- /dev/null +++ b/test/fuzz/fuzz-json/leak-normalize-object @@ -0,0 +1 @@ +[7,7,7,7,{"":7,"":7,"^t":7,"-":7},2777,7,7,7,3] \ No newline at end of file From 99b1145aae682ddd7554c7e3ac5ebf778e88f87d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 May 2022 10:51:43 +0200 Subject: [PATCH 11/11] shared/json: fix memleak in sort --- src/shared/json.c | 4 ++-- test/fuzz/fuzz-json/leak-sort | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 test/fuzz/fuzz-json/leak-sort diff --git a/src/shared/json.c b/src/shared/json.c index d9eda88b12..bcc109abc2 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4581,7 +4581,7 @@ static int json_cmp_strings(const void *x, const void *y) { int json_variant_sort(JsonVariant **v) { _cleanup_free_ JsonVariant **a = NULL; - JsonVariant *n = NULL; + _cleanup_(json_variant_unrefp) JsonVariant *n = NULL; size_t m; int r; @@ -4614,7 +4614,7 @@ int json_variant_sort(JsonVariant **v) { if (!n->sorted) /* Check if this worked. This will fail if there are multiple identical keys used. */ return -ENOTUNIQ; - JSON_VARIANT_REPLACE(*v, n); + JSON_VARIANT_REPLACE(*v, TAKE_PTR(n)); return 1; } diff --git a/test/fuzz/fuzz-json/leak-sort b/test/fuzz/fuzz-json/leak-sort new file mode 100644 index 0000000000..f8446dbdc7 --- /dev/null +++ b/test/fuzz/fuzz-json/leak-sort @@ -0,0 +1 @@ +{"":2,"":6,"-":7} \ No newline at end of file