Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent mode is broken on aarch64, since AFL_QEMU_TARGET_ARM64_SNIPPET is inserted before updating pc #20

Closed
galli-leo opened this issue Apr 14, 2021 · 5 comments
Assignees

Comments

@galli-leo
Copy link

galli-leo commented Apr 14, 2021

I investigated an issue I had with persistent mode on aarch64, where a bunch of weird stuff would happen.

I finally figured out what causes this, in translate-a64.c AFL_QEMU_TARGET_ARM64_SNIPPET is referenced before the pc is updated (s->pc_curr = s->base.pc_next;).
In all other architectures, the pc is first updated, then the snippet is referenced. This leads to the persistent loop starting one instruction too late, and hence fp, lr are not preserved (at least for my binary).

Since I don't know where exactly it should come afterwards, I didn't open a PR, but if it's just the line afterwards like on arm, I can do that.

A repro case is here: https://gist.github.com/galli-leo/c879a84e5512f3b90d63b1e28f11112a
hook.c is not really important, just that one is needed. repro_fuzz.c should be compiled for aarch64.

Here is also the qemu log for the (beginning) of the do_fuzz function:

IN: do_fuzz
0x00400614:  a9bd7bfd  stp      x29, x30, [sp, #-0x30]!
0x00400618:  910003fd  mov      x29, sp
0x0040061c:  f90017e0  str      x0, [sp, #0x28]
0x00400620:  f90013e1  str      x1, [sp, #0x20]
0x00400624:  f9000fe2  str      x2, [sp, #0x18]
0x00400628:  f9400fe3  ldr      x3, [sp, #0x18]
0x0040062c:  f94013e2  ldr      x2, [sp, #0x20]
0x00400630:  f94017e1  ldr      x1, [sp, #0x28]
0x00400634:  90000000  adrp     x0, #0x400000
0x00400638:  911d2000  add      x0, x0, #0x748
0x0040063c:  97ffffa9  bl       #0x4004e0

OP:
 mov_i64 tmp0,$0x1461
 call afl_maybe_log,$0x1,$0,tmp0
 ld_i32 tmp2,env,$0xfffffffffffffff0
 brcond_i32 tmp2,$0x0,lt,$L0

 ---- 0000000000400614 0000000000000000 0000000000000000
 mov_i64 tmp0,sp
 add_i64 tmp0,tmp0,$0xffffffffffffffd0
 shl_i64 tmp5,tmp0,$0x8
 sar_i64 tmp5,tmp5,$0x8
 and_i64 tmp5,tmp5,tmp0
 qemu_st_i64 x29,tmp5,leq,8
 add_i64 tmp5,tmp5,$0x8
 qemu_st_i64 lr,tmp5,leq,8
 mov_i64 sp,tmp0

 ---- 0000000000400618 0000000000000000 0000000000000000
 call afl_persistent_routine,$0x1,$0,env
 mov_i64 tmp0,sp
 mov_i64 x29,tmp0
@andreafioraldi
Copy link
Member

Hi @galli-leo ,
yes the pc update is in the wrong place, probably something went wrong while updating the patches.
Can you try to just move AFL_QEMU_TARGET_ARM64_SNIPPET the line before if (dc_isar_feature(aa64_bti, s)) { and tell me if this fix the issue?

@andreafioraldi
Copy link
Member

You can rebuild qemuafl from afl++ keeping your changes using NO_CHECKOUT=1 when calling the build shell script

@galli-leo
Copy link
Author

Yep works correctly!

@vanhauser-thc
Copy link
Member

@galli-leo do you want to push a PR?

@andreafioraldi
Copy link
Member

Nevermind, I did it

Dil4rd pushed a commit to Dil4rd/qemuafl that referenced this issue Jul 26, 2021
When the reconnect in NBD client is in progress, the iochannel used for
NBD connection doesn't exist.  Therefore an attempt to detach it from
the aio_context of the parent BlockDriverState results in a NULL pointer
dereference.

The problem is triggerable, in particular, when an outgoing migration is
about to finish, and stopping the dataplane tries to move the
BlockDriverState from the iothread aio_context to the main loop.  If the
NBD connection is lost before this point, and the NBD client has entered
the reconnect procedure, QEMU crashes:

  #0  qemu_aio_coroutine_enter (ctx=0x5618056c7580, co=0x0)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/qemu-coroutine.c:109
  AFLplusplus#1  0x00005618034b1b68 in nbd_client_attach_aio_context_bh (
      opaque=0x561805ed4c00) at /build/qemu-6MF7tq/qemu-5.0.1/block/nbd.c:164
  AFLplusplus#2  0x000056180353116b in aio_wait_bh (opaque=0x7f60e1e63700)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/aio-wait.c:55
  AFLplusplus#3  0x0000561803530633 in aio_bh_call (bh=0x7f60d40a7e80)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/async.c:136
  AFLplusplus#4  aio_bh_poll (ctx=ctx@entry=0x5618056c7580)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/async.c:164
  AFLplusplus#5  0x0000561803533e5a in aio_poll (ctx=ctx@entry=0x5618056c7580,
      blocking=blocking@entry=true)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/aio-posix.c:650
  AFLplusplus#6  0x000056180353128d in aio_wait_bh_oneshot (ctx=0x5618056c7580,
      cb=<optimized out>, opaque=<optimized out>)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/aio-wait.c:71
  AFLplusplus#7  0x000056180345c50a in bdrv_attach_aio_context (new_context=0x5618056c7580,
      bs=0x561805ed4c00) at /build/qemu-6MF7tq/qemu-5.0.1/block.c:6172
  AFLplusplus#8  bdrv_set_aio_context_ignore (bs=bs@entry=0x561805ed4c00,
      new_context=new_context@entry=0x5618056c7580,
      ignore=ignore@entry=0x7f60e1e63780)
      at /build/qemu-6MF7tq/qemu-5.0.1/block.c:6237
  AFLplusplus#9  0x000056180345c969 in bdrv_child_try_set_aio_context (
      bs=bs@entry=0x561805ed4c00, ctx=0x5618056c7580,
      ignore_child=<optimized out>, errp=<optimized out>)
      at /build/qemu-6MF7tq/qemu-5.0.1/block.c:6332
  AFLplusplus#10 0x00005618034957db in blk_do_set_aio_context (blk=0x56180695b3f0,
      new_context=0x5618056c7580, update_root_node=update_root_node@entry=true,
      errp=errp@entry=0x0)
      at /build/qemu-6MF7tq/qemu-5.0.1/block/block-backend.c:1989
  AFLplusplus#11 0x00005618034980bd in blk_set_aio_context (blk=<optimized out>,
      new_context=<optimized out>, errp=errp@entry=0x0)
      at /build/qemu-6MF7tq/qemu-5.0.1/block/block-backend.c:2010
  AFLplusplus#12 0x0000561803197953 in virtio_blk_data_plane_stop (vdev=<optimized out>)
      at /build/qemu-6MF7tq/qemu-5.0.1/hw/block/dataplane/virtio-blk.c:292
  AFLplusplus#13 0x00005618033d67bf in virtio_bus_stop_ioeventfd (bus=0x5618056d9f08)
      at /build/qemu-6MF7tq/qemu-5.0.1/hw/virtio/virtio-bus.c:245
  AFLplusplus#14 0x00005618031c9b2e in virtio_vmstate_change (opaque=0x5618056d9f90,
      running=0, state=<optimized out>)
      at /build/qemu-6MF7tq/qemu-5.0.1/hw/virtio/virtio.c:3220
  AFLplusplus#15 0x0000561803208bfd in vm_state_notify (running=running@entry=0,
      state=state@entry=RUN_STATE_FINISH_MIGRATE)
      at /build/qemu-6MF7tq/qemu-5.0.1/softmmu/vl.c:1275
  AFLplusplus#16 0x0000561803155c02 in do_vm_stop (state=RUN_STATE_FINISH_MIGRATE,
      send_stop=<optimized out>) at /build/qemu-6MF7tq/qemu-5.0.1/cpus.c:1032
  AFLplusplus#17 0x00005618033e3765 in migration_completion (s=0x5618056e6960)
      at /build/qemu-6MF7tq/qemu-5.0.1/migration/migration.c:2914
  AFLplusplus#18 migration_iteration_run (s=0x5618056e6960)
      at /build/qemu-6MF7tq/qemu-5.0.1/migration/migration.c:3275
  AFLplusplus#19 migration_thread (opaque=opaque@entry=0x5618056e6960)
      at /build/qemu-6MF7tq/qemu-5.0.1/migration/migration.c:3439
  AFLplusplus#20 0x0000561803536ad6 in qemu_thread_start (args=<optimized out>)
      at /build/qemu-6MF7tq/qemu-5.0.1/util/qemu-thread-posix.c:519
  AFLplusplus#21 0x00007f61085d06ba in start_thread ()
     from /lib/x86_64-linux-gnu/libpthread.so.0
  AFLplusplus#22 0x00007f610830641d in sysctl () from /lib/x86_64-linux-gnu/libc.so.6
  AFLplusplus#23 0x0000000000000000 in ?? ()

Fix it by checking that the iochannel is non-null before trying to
detach it from the aio_context.  If it is null, no detaching is needed,
and it will get reattached in the proper aio_context once the connection
is reestablished.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210129073859.683063-2-rvkagan@yandex-team.ru>
Signed-off-by: Eric Blake <eblake@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants