Message ID | 25ff34ee279d98866add04ca5a7dbf4c9ea47999.1358611253.git.blauwirbel@gmail.com |
---|---|
State | New |
Headers | show |
Am 19.01.2013 17:01, schrieb Blue Swirl: > Perform input tests on random data. > > Improvement to code coverage for qapi/string-input-visitor.c > is about 3 percentage points. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> Does this test pass for you? It consistently segfaults for me. /string-visitor/input/fuzz: ==30703== Conditional jump or move depends on uninitialised value(s) ==30703== at 0x508E738: g_free (gmem.c:262) ==30703== by 0x10B123: test_visitor_in_fuzz (test-string-input-visitor.c:207) ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) ==30703== ==30703== Conditional jump or move depends on uninitialised value(s) ==30703== at 0x4A055B4: free (vg_replace_malloc.c:366) ==30703== by 0x508E742: g_free (gmem.c:263) ==30703== by 0x10B123: test_visitor_in_fuzz (test-string-input-visitor.c:207) ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) ==30703== ==30703== Invalid free() / delete / delete[] ==30703== at 0x4A055FE: free (vg_replace_malloc.c:366) ==30703== by 0x508E742: g_free (gmem.c:263) ==30703== by 0x10B123: test_visitor_in_fuzz (test-string-input-visitor.c:207) ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) ==30703== Address 0x2102508021024020 is not stack'd, malloc'd or (recently) free'd ==30703==
On Wed, Jan 30, 2013 at 4:37 PM, Kevin Wolf <kwolf@redhat.com> wrote: > Am 19.01.2013 17:01, schrieb Blue Swirl: >> Perform input tests on random data. >> >> Improvement to code coverage for qapi/string-input-visitor.c >> is about 3 percentage points. >> >> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > > Does this test pass for you? It consistently segfaults for me. Yes, it works on x86_64, i386, arm and sparc64. > > /string-visitor/input/fuzz: ==30703== Conditional jump or move depends > on uninitialised value(s) > ==30703== at 0x508E738: g_free (gmem.c:262) > ==30703== by 0x10B123: test_visitor_in_fuzz > (test-string-input-visitor.c:207) > ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) > ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) > ==30703== > ==30703== Conditional jump or move depends on uninitialised value(s) > ==30703== at 0x4A055B4: free (vg_replace_malloc.c:366) > ==30703== by 0x508E742: g_free (gmem.c:263) > ==30703== by 0x10B123: test_visitor_in_fuzz > (test-string-input-visitor.c:207) > ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) > ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) > ==30703== > ==30703== Invalid free() / delete / delete[] > ==30703== at 0x4A055FE: free (vg_replace_malloc.c:366) > ==30703== by 0x508E742: g_free (gmem.c:263) > ==30703== by 0x10B123: test_visitor_in_fuzz > (test-string-input-visitor.c:207) > ==30703== by 0x50ABCA7: g_test_run_suite_internal (gtestutils.c:1174) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50ABE15: g_test_run_suite_internal (gtestutils.c:1233) > ==30703== by 0x50AC10E: g_test_run_suite (gtestutils.c:1282) > ==30703== by 0x108FBF: main (test-string-input-visitor.c:242) > ==30703== Address 0x2102508021024020 is not stack'd, malloc'd or > (recently) free'd > ==30703== The call to g_free() in the fuzz function looks suspect. I used test_visitor_in_string() as a model (which seems to have been copied from test-qmp-input-visitor.c), is the call to g_free() correct there either? Perhaps Paolo or Luiz would know?
On 2 February 2013 12:40, Blue Swirl <blauwirbel@gmail.com> wrote: > On Wed, Jan 30, 2013 at 4:37 PM, Kevin Wolf <kwolf@redhat.com> wrote: >> Am 19.01.2013 17:01, schrieb Blue Swirl: >>> Perform input tests on random data. >>> >>> Improvement to code coverage for qapi/string-input-visitor.c >>> is about 3 percentage points. >>> >>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >> >> Does this test pass for you? It consistently segfaults for me. > > Yes, it works on x86_64, i386, arm and sparc64. > The call to g_free() in the fuzz function looks suspect. On MacOSX this g_free fails the malloc system's checks: /string-visitor/input/fuzz: test-string-input-visitor(76691) malloc: *** error for object 0x7fff8e606b00: pointer being freed was not allocated This happens because you're reusing the Error* without checking or clearing it after each call. If it's handed an Error** that indicates an error has already occurred, visit_type_str() does nothing, and so in test_visitor_in_fuzz() nothing has set sres, and we try to g_free() an uninitialized pointer. This code should either: (a) avoid passing the visitors anything that could provoke an error, and g_assert(!error_is_set(&errp)) after each call (b) if errors are ok, do if (error_is_set(&errp)) { error_free(errp); errp = NULL; } after each call. I don't know exactly what the semantics of visit_type_str() are [my guess is "if no error, caller must g_free() string, otherwise no string allocated"] -- if somebody who did know was able to write some brief docstring comments for visitor.h that might be nice :-) -- PMM
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 899feda..f6b0093 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -165,6 +165,53 @@ static void test_visitor_in_enum(TestInputVisitorData *data, data->siv = NULL; } +/* Try to crash the visitors */ +static void test_visitor_in_fuzz(TestInputVisitorData *data, + const void *unused) +{ + int64_t ires; + bool bres; + double nres; + char *sres; + EnumOne eres; + Error *errp = NULL; + Visitor *v; + unsigned int i; + char buf[10000]; + + for (i = 0; i < 100; i++) { + unsigned int j; + + j = g_test_rand_int_range(0, sizeof(buf) - 1); + + buf[j] = '\0'; + + if (j != 0) { + for (j--; j != 0; j--) { + buf[j - 1] = (char)g_test_rand_int_range(0, 256); + } + } + + v = visitor_input_test_init(data, buf); + visit_type_int(v, &ires, NULL, &errp); + + v = visitor_input_test_init(data, buf); + visit_type_bool(v, &bres, NULL, &errp); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, buf); + visit_type_number(v, &nres, NULL, &errp); + + v = visitor_input_test_init(data, buf); + visit_type_str(v, &sres, NULL, &errp); + g_free(sres); + + v = visitor_input_test_init(data, buf); + visit_type_EnumOne(v, &eres, NULL, &errp); + visitor_input_teardown(data, NULL); + } +} + static void input_visitor_test_add(const char *testpath, TestInputVisitorData *data, void (*test_func)(TestInputVisitorData *data, const void *user_data)) @@ -189,6 +236,8 @@ int main(int argc, char **argv) &in_visitor_data, test_visitor_in_string); input_visitor_test_add("/string-visitor/input/enum", &in_visitor_data, test_visitor_in_enum); + input_visitor_test_add("/string-visitor/input/fuzz", + &in_visitor_data, test_visitor_in_fuzz); g_test_run();
Perform input tests on random data. Improvement to code coverage for qapi/string-input-visitor.c is about 3 percentage points. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- tests/test-string-input-visitor.c | 49 +++++++++++++++++++++++++++++++++++++ 1 files changed, 49 insertions(+), 0 deletions(-)