diff mbox series

[v4,3/3] fuzz: make object-name matching case-insensitive

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

Commit Message

Alexander Bulekov June 28, 2021, 5:23 a.m. UTC
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(-)

Comments

Alexander Bulekov June 29, 2021, 1:11 a.m. UTC | #1
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
>
Darren Kenny June 29, 2021, 10:03 a.m. UTC | #2
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
Darren Kenny June 29, 2021, 10:07 a.m. UTC | #3
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 mbox series

Patch

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");