[PATCH] fuzz: don't leave orphan llvm-symbolizers around

Alexander Bulekov posted 1 patch 4 years, 8 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210310061236.947182-1-alxndr@bu.edu
Maintainers: Laurent Vivier <lvivier@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Thomas Huth <thuth@redhat.com>
tests/qtest/fuzz/generic_fuzz.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
[PATCH] fuzz: don't leave orphan llvm-symbolizers around
Posted by Alexander Bulekov 4 years, 8 months ago
I noticed that with a sufficiently small timeout, the fuzzer fork-server
sometimes locks up. On closer inspection, the issue appeared to be
caused by entering our SIGALRM handler, while libfuzzer is in it's crash
handlers. Because libfuzzer relies on pipe communication with an
external child process to print out stack-traces, we shouldn't exit
early, and leave an orphan child. Check for children in the SIGALRM
handler to avoid this issue.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/generic_fuzz.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index ee8c17a04c..387ae2020a 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -583,6 +583,21 @@ static void handle_timeout(int sig)
         fprintf(stderr, "[Timeout]\n");
         fflush(stderr);
     }
+
+    /*
+     * If there is a crash, libfuzzer/ASAN forks a child to run an
+     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
+     * communicates with this child using a pipe.  If we timeout+Exit, while
+     * libfuzzer is still communicating with the llvm-symbolizer child, we will
+     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
+     * to lead to a deadlock in the forkserver. Use waitpid to check if there
+     * are any waitable children. If so, exit out of the signal-handler, and
+     * let libfuzzer finish communicating with the child, and exit, on its own.
+     */
+    if (waitpid(-1, NULL, WNOHANG) == 0) {
+        return;
+    }
+
     _Exit(0);
 }
 
-- 
2.28.0


Re: [PATCH] fuzz: don't leave orphan llvm-symbolizers around
Posted by Thomas Huth 4 years, 8 months ago
On 10/03/2021 07.12, Alexander Bulekov wrote:
> I noticed that with a sufficiently small timeout, the fuzzer fork-server
> sometimes locks up. On closer inspection, the issue appeared to be
> caused by entering our SIGALRM handler, while libfuzzer is in it's crash
> handlers. Because libfuzzer relies on pipe communication with an
> external child process to print out stack-traces, we shouldn't exit
> early, and leave an orphan child. Check for children in the SIGALRM
> handler to avoid this issue.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/qtest/fuzz/generic_fuzz.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index ee8c17a04c..387ae2020a 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -583,6 +583,21 @@ static void handle_timeout(int sig)
>           fprintf(stderr, "[Timeout]\n");
>           fflush(stderr);
>       }
> +
> +    /*
> +     * If there is a crash, libfuzzer/ASAN forks a child to run an
> +     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
> +     * communicates with this child using a pipe.  If we timeout+Exit, while
> +     * libfuzzer is still communicating with the llvm-symbolizer child, we will
> +     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
> +     * to lead to a deadlock in the forkserver. Use waitpid to check if there
> +     * are any waitable children. If so, exit out of the signal-handler, and
> +     * let libfuzzer finish communicating with the child, and exit, on its own.
> +     */
> +    if (waitpid(-1, NULL, WNOHANG) == 0) {
> +        return;
> +    }
> +
>       _Exit(0);
>   }

Acked-by: Thomas Huth <thuth@redhat.com>


Re: [PATCH] fuzz: don't leave orphan llvm-symbolizers around
Posted by Darren Kenny 4 years, 8 months ago
On Wednesday, 2021-03-10 at 01:12:36 -05, Alexander Bulekov wrote:
> I noticed that with a sufficiently small timeout, the fuzzer fork-server
> sometimes locks up. On closer inspection, the issue appeared to be
> caused by entering our SIGALRM handler, while libfuzzer is in it's crash
> handlers. Because libfuzzer relies on pipe communication with an
> external child process to print out stack-traces, we shouldn't exit
> early, and leave an orphan child. Check for children in the SIGALRM
> handler to avoid this issue.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index ee8c17a04c..387ae2020a 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -583,6 +583,21 @@ static void handle_timeout(int sig)
>          fprintf(stderr, "[Timeout]\n");
>          fflush(stderr);
>      }
> +
> +    /*
> +     * If there is a crash, libfuzzer/ASAN forks a child to run an
> +     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
> +     * communicates with this child using a pipe.  If we timeout+Exit, while
> +     * libfuzzer is still communicating with the llvm-symbolizer child, we will
> +     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
> +     * to lead to a deadlock in the forkserver. Use waitpid to check if there
> +     * are any waitable children. If so, exit out of the signal-handler, and
> +     * let libfuzzer finish communicating with the child, and exit, on its own.
> +     */
> +    if (waitpid(-1, NULL, WNOHANG) == 0) {
> +        return;
> +    }
> +
>      _Exit(0);
>  }
>  
> -- 
> 2.28.0