[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] fuzz: do not use POSIX shm for coverage bitmap
From: |
Darren Kenny |
Subject: |
Re: [PATCH] fuzz: do not use POSIX shm for coverage bitmap |
Date: |
Tue, 23 Jun 2020 09:03:19 +0100 |
Hi Alex,
On Monday, 2020-06-22 at 12:50:40 -04, Alexander Bulekov wrote:
> We used shm_open with mmap to share libfuzzer's coverage bitmap with
> child (runner) processes. The same functionality can be achieved with
> MAP_SHARED | MAP_ANONYMOUS, since we do not care about naming or
> permissioning the shared memory object.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> This might fix:
> qemu-fuzz-i386-target-virtio-net-socket: Unexpected-exit in
> counter_shm_init
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=23636 (private link)
>
> oss-fuzz does not provide access to /dev/, so it is likely that shm_open
> breaks, when it tries to access /dev/shm. This seems likely, based on
> the oss-fuzz minijail setup:
> https://github.com/google/oss-fuzz/blob/3740c751fd9edea138c17783995d370d6b1b89bc/infra/base-images/base-runner/run_minijail
>
> tests/qtest/fuzz/fork_fuzz.c | 40 ++++++++++++------------------------
> 1 file changed, 13 insertions(+), 27 deletions(-)
>
> diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
> index 2bd0851903..6ffb2a7937 100644
> --- a/tests/qtest/fuzz/fork_fuzz.c
> +++ b/tests/qtest/fuzz/fork_fuzz.c
> @@ -17,39 +17,25 @@
>
> void counter_shm_init(void)
> {
> - char *shm_path = g_strdup_printf("/qemu-fuzz-cntrs.%d", getpid());
> - int fd = shm_open(shm_path, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
> - g_free(shm_path);
> -
> - if (fd == -1) {
> - perror("Error: ");
> - exit(1);
> - }
> - if (ftruncate(fd, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START) == -1) {
> - perror("Error: ");
> - exit(1);
> - }
> - /* Copy what's in the counter region to the shm.. */
> - void *rptr = mmap(NULL ,
> - &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> - PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - memcpy(rptr,
> + /* Copy what's in the counter region to a temporary buffer.. */
> + void *copy = malloc(&__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> + memcpy(copy,
> &__FUZZ_COUNTERS_START,
> &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
>
> - munmap(rptr, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -
> - /* And map the shm over the counter region */
> - rptr = mmap(&__FUZZ_COUNTERS_START,
> - &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> - PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, fd, 0);
> -
> - close(fd);
> -
> - if (!rptr) {
> + /* Map a shared region over the counter region */
> + if (mmap(&__FUZZ_COUNTERS_START,
> + &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED | MAP_ANONYMOUS,
> + 0, 0) == MAP_FAILED) {
It's not really necessary I guess, but for completeness you might want
to free(copy) here too.
Otherwise, this looks good, so:
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Thanks,
Darren
> perror("Error: ");
> exit(1);
> }
> +
> + /* Copy the original data back to the counter-region */
> + memcpy(&__FUZZ_COUNTERS_START, copy,
> + &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> + free(copy);
> }
>
>
> --
> 2.26.2