| Message ID | 20210628052349.113262-4-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fuzzer pattern-matching, timeouts, and instrumentation-filtering | expand |
On 210628 0123, Alexander Bulekov wrote: > We have some configs for devices such as the AC97 and ES1370 that were > not matching memory-regions correctly, because the configs provided > lowercase names. To resolve these problems and prevent them from > occurring again in the future, convert both the pattern and names to > lower-case, prior to checking for a match. > > Suggested-by: Darren Kenny <darren.kenny@oracle.com> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > --- > tests/qtest/fuzz/generic_fuzz.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c > index 71d36e8f6f..0695a349b2 100644 > --- a/tests/qtest/fuzz/generic_fuzz.c > +++ b/tests/qtest/fuzz/generic_fuzz.c > @@ -751,8 +751,13 @@ static int locate_fuzz_memory_regions(Object *child, void *opaque) > > static int locate_fuzz_objects(Object *child, void *opaque) > { > + GString *type_name; > + GString *path_name; > char *pattern = opaque; > - if (g_pattern_match_simple(pattern, object_get_typename(child))) { > + > + type_name = g_string_new(object_get_typename(child)); > + g_string_ascii_down(type_name); > + if (g_pattern_match_simple(pattern, type_name->str)) { > /* Find and save ptrs to any child MemoryRegions */ > object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); > > @@ -769,8 +774,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) > g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child)); > } > } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { > - if (g_pattern_match_simple(pattern, > - object_get_canonical_path_component(child))) { > + path_name = g_string_new(object_get_canonical_path_component(child)); > + g_string_ascii_down(path_name); > + if (g_pattern_match_simple(pattern, path_name->str)) { > MemoryRegion *mr; > mr = MEMORY_REGION(child); > if ((memory_region_is_ram(mr) || > @@ -779,7 +785,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) > g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); > } > } > + g_string_free(path_name, true); > } > + g_string_free(type_name, true); > return 0; > } > > @@ -807,6 +815,7 @@ static void generic_pre_fuzz(QTestState *s) > MemoryRegion *mr; > QPCIBus *pcibus; > char **result; > + GString *pattern; ^ Just noticed that this collides with struct pattern through a sizeof(pattern) call below, causing nasty heap issues during fuzzing. I'll send a v4 with a better name. -Alex > > if (!getenv("QEMU_FUZZ_OBJECTS")) { > usage(); > @@ -836,10 +845,17 @@ static void generic_pre_fuzz(QTestState *s) > > result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1); > for (int i = 0; result[i] != NULL; i++) { > + pattern = g_string_new(result[i]); > + /* > + * Make the pattern lowercase. We do the same for all the MemoryRegion > + * and Type names so the configs are case-insensitive. > + */ > + g_string_ascii_down(pattern); > printf("Matching objects by name %s\n", result[i]); > object_child_foreach_recursive(qdev_get_machine(), > locate_fuzz_objects, > - result[i]); > + pattern->str); > + g_string_free(pattern, true); > } > g_strfreev(result); > printf("This process will try to fuzz the following MemoryRegions:\n"); > -- > 2.28.0 >
Hi Alex, On Monday, 2021-06-28 at 01:23:49 -04, Alexander Bulekov wrote: > We have some configs for devices such as the AC97 and ES1370 that were > not matching memory-regions correctly, because the configs provided > lowercase names. To resolve these problems and prevent them from > occurring again in the future, convert both the pattern and names to > lower-case, prior to checking for a match. > > Suggested-by: Darren Kenny <darren.kenny@oracle.com> Thanks for doing this, LGTM. > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Thanks, Darren. > --- > tests/qtest/fuzz/generic_fuzz.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c > index 71d36e8f6f..0695a349b2 100644 > --- a/tests/qtest/fuzz/generic_fuzz.c > +++ b/tests/qtest/fuzz/generic_fuzz.c > @@ -751,8 +751,13 @@ static int locate_fuzz_memory_regions(Object *child, void *opaque) > > static int locate_fuzz_objects(Object *child, void *opaque) > { > + GString *type_name; > + GString *path_name; > char *pattern = opaque; > - if (g_pattern_match_simple(pattern, object_get_typename(child))) { > + > + type_name = g_string_new(object_get_typename(child)); > + g_string_ascii_down(type_name); > + if (g_pattern_match_simple(pattern, type_name->str)) { > /* Find and save ptrs to any child MemoryRegions */ > object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); > > @@ -769,8 +774,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) > g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child)); > } > } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { > - if (g_pattern_match_simple(pattern, > - object_get_canonical_path_component(child))) { > + path_name = g_string_new(object_get_canonical_path_component(child)); > + g_string_ascii_down(path_name); > + if (g_pattern_match_simple(pattern, path_name->str)) { > MemoryRegion *mr; > mr = MEMORY_REGION(child); > if ((memory_region_is_ram(mr) || > @@ -779,7 +785,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) > g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); > } > } > + g_string_free(path_name, true); > } > + g_string_free(type_name, true); > return 0; > } > > @@ -807,6 +815,7 @@ static void generic_pre_fuzz(QTestState *s) > MemoryRegion *mr; > QPCIBus *pcibus; > char **result; > + GString *pattern; > > if (!getenv("QEMU_FUZZ_OBJECTS")) { > usage(); > @@ -836,10 +845,17 @@ static void generic_pre_fuzz(QTestState *s) > > result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1); > for (int i = 0; result[i] != NULL; i++) { > + pattern = g_string_new(result[i]); > + /* > + * Make the pattern lowercase. We do the same for all the MemoryRegion > + * and Type names so the configs are case-insensitive. > + */ > + g_string_ascii_down(pattern); > printf("Matching objects by name %s\n", result[i]); > object_child_foreach_recursive(qdev_get_machine(), > locate_fuzz_objects, > - result[i]); > + pattern->str); > + g_string_free(pattern, true); > } > g_strfreev(result); > printf("This process will try to fuzz the following MemoryRegions:\n"); > -- > 2.28.0
On Monday, 2021-06-28 at 21:11:51 -04, Alexander Bulekov wrote: > On 210628 0123, Alexander Bulekov wrote: >> We have some configs for devices such as the AC97 and ES1370 that were >> not matching memory-regions correctly, because the configs provided >> lowercase names. To resolve these problems and prevent them from >> occurring again in the future, convert both the pattern and names to >> lower-case, prior to checking for a match. >> >> Suggested-by: Darren Kenny <darren.kenny@oracle.com> >> Signed-off-by: Alexander Bulekov <alxndr@bu.edu> >> --- >> tests/qtest/fuzz/generic_fuzz.c | 24 ++++++++++++++++++++---- >> 1 file changed, 20 insertions(+), 4 deletions(-) >> >> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c >> index 71d36e8f6f..0695a349b2 100644 >> --- a/tests/qtest/fuzz/generic_fuzz.c >> +++ b/tests/qtest/fuzz/generic_fuzz.c >> @@ -751,8 +751,13 @@ static int locate_fuzz_memory_regions(Object *child, void *opaque) >> >> static int locate_fuzz_objects(Object *child, void *opaque) >> { >> + GString *type_name; >> + GString *path_name; >> char *pattern = opaque; >> - if (g_pattern_match_simple(pattern, object_get_typename(child))) { >> + >> + type_name = g_string_new(object_get_typename(child)); >> + g_string_ascii_down(type_name); >> + if (g_pattern_match_simple(pattern, type_name->str)) { >> /* Find and save ptrs to any child MemoryRegions */ >> object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); >> >> @@ -769,8 +774,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) >> g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child)); >> } >> } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { >> - if (g_pattern_match_simple(pattern, >> - object_get_canonical_path_component(child))) { >> + path_name = g_string_new(object_get_canonical_path_component(child)); >> + g_string_ascii_down(path_name); >> + if (g_pattern_match_simple(pattern, path_name->str)) { >> MemoryRegion *mr; >> mr = MEMORY_REGION(child); >> if ((memory_region_is_ram(mr) || >> @@ -779,7 +785,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) >> g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); >> } >> } >> + g_string_free(path_name, true); >> } >> + g_string_free(type_name, true); >> return 0; >> } >> >> @@ -807,6 +815,7 @@ static void generic_pre_fuzz(QTestState *s) >> MemoryRegion *mr; >> QPCIBus *pcibus; >> char **result; >> + GString *pattern; > ^ > Just noticed that this collides with struct pattern through a > sizeof(pattern) call below, causing nasty heap issues during fuzzing. > I'll send a v4 with a better name. > Certainly wasn't obvious from the diff :) Fair enough. Thanks, Darren.
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 71d36e8f6f..0695a349b2 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -751,8 +751,13 @@ static int locate_fuzz_memory_regions(Object *child, void *opaque) static int locate_fuzz_objects(Object *child, void *opaque) { + GString *type_name; + GString *path_name; char *pattern = opaque; - if (g_pattern_match_simple(pattern, object_get_typename(child))) { + + type_name = g_string_new(object_get_typename(child)); + g_string_ascii_down(type_name); + if (g_pattern_match_simple(pattern, type_name->str)) { /* Find and save ptrs to any child MemoryRegions */ object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); @@ -769,8 +774,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child)); } } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { - if (g_pattern_match_simple(pattern, - object_get_canonical_path_component(child))) { + path_name = g_string_new(object_get_canonical_path_component(child)); + g_string_ascii_down(path_name); + if (g_pattern_match_simple(pattern, path_name->str)) { MemoryRegion *mr; mr = MEMORY_REGION(child); if ((memory_region_is_ram(mr) || @@ -779,7 +785,9 @@ static int locate_fuzz_objects(Object *child, void *opaque) g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); } } + g_string_free(path_name, true); } + g_string_free(type_name, true); return 0; } @@ -807,6 +815,7 @@ static void generic_pre_fuzz(QTestState *s) MemoryRegion *mr; QPCIBus *pcibus; char **result; + GString *pattern; if (!getenv("QEMU_FUZZ_OBJECTS")) { usage(); @@ -836,10 +845,17 @@ static void generic_pre_fuzz(QTestState *s) result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1); for (int i = 0; result[i] != NULL; i++) { + pattern = g_string_new(result[i]); + /* + * Make the pattern lowercase. We do the same for all the MemoryRegion + * and Type names so the configs are case-insensitive. + */ + g_string_ascii_down(pattern); printf("Matching objects by name %s\n", result[i]); object_child_foreach_recursive(qdev_get_machine(), locate_fuzz_objects, - result[i]); + pattern->str); + g_string_free(pattern, true); } g_strfreev(result); printf("This process will try to fuzz the following MemoryRegions:\n");
We have some configs for devices such as the AC97 and ES1370 that were not matching memory-regions correctly, because the configs provided lowercase names. To resolve these problems and prevent them from occurring again in the future, convert both the pattern and names to lower-case, prior to checking for a match. Suggested-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Alexander Bulekov <alxndr@bu.edu> --- tests/qtest/fuzz/generic_fuzz.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)