Skip to content
Snippets Groups Projects
  • Amit Shah's avatar
    30fb2ca6
    balloon: Separate out stat and balloon handling · 30fb2ca6
    Amit Shah authored
    
    Passing on '0' as ballooning target to indicate retrieval of stats is
    bad API.  It also makes 'balloon 0' in the monitor cause a segfault.
    Have two different functions handle the different functionality instead.
    
    Detailed explanation from Markus's review:
    
    1. do_info_balloon() is an info_async() method.  It receives a callback
       with argument, to be called exactly once (callback frees the
       argument).  It passes the callback via qemu_balloon_status() and
       indirectly through qemu_balloon_event to virtio_balloon_to_target().
    
       virtio_balloon_to_target() executes its balloon stats half.  It
       stores the callback in the device state.
    
       If it can't send a stats request, it resets stats and calls the
       callback right away.
    
       Else, it sends a stats request.  The device model runs the callback
       when it receives the answer.
    
       Works.
    
    2. do_balloon() is a cmd_async() method.  It receives a callback with
       argument, to be called when the command completes.  do_balloon()
       calls it right before it succeeds.  Odd, but should work.
    
       Nevertheless, it passes the callback on via qemu_ballon() and
       indirectly through qemu_balloon_event to virtio_balloon_to_target().
    
       a. If the argument is non-zero, virtio_balloon_to_target() executes
          its balloon half, which doesn't use the callback in any way.
    
          Odd, but works.
    
       b. If the argument is zero, virtio_balloon_to_target() executes its
          balloon stats half, just like in 1.  It either calls the callback
          right away, or arranges for it to be called later.
    
          Thus, the callback runs twice: use after free and double free.
    
    Test case: start with -S -device virtio-balloon, execute "balloon 0" in
    human monitor.  Runs the callback first from virtio_balloon_to_target(),
    then again from do_balloon().
    
    Reported-by: default avatarMike Cao <bcao@redhat.com>
    Signed-off-by: default avatarAmit Shah <amit.shah@redhat.com>
    Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
    30fb2ca6
    History
    balloon: Separate out stat and balloon handling
    Amit Shah authored
    
    Passing on '0' as ballooning target to indicate retrieval of stats is
    bad API.  It also makes 'balloon 0' in the monitor cause a segfault.
    Have two different functions handle the different functionality instead.
    
    Detailed explanation from Markus's review:
    
    1. do_info_balloon() is an info_async() method.  It receives a callback
       with argument, to be called exactly once (callback frees the
       argument).  It passes the callback via qemu_balloon_status() and
       indirectly through qemu_balloon_event to virtio_balloon_to_target().
    
       virtio_balloon_to_target() executes its balloon stats half.  It
       stores the callback in the device state.
    
       If it can't send a stats request, it resets stats and calls the
       callback right away.
    
       Else, it sends a stats request.  The device model runs the callback
       when it receives the answer.
    
       Works.
    
    2. do_balloon() is a cmd_async() method.  It receives a callback with
       argument, to be called when the command completes.  do_balloon()
       calls it right before it succeeds.  Odd, but should work.
    
       Nevertheless, it passes the callback on via qemu_ballon() and
       indirectly through qemu_balloon_event to virtio_balloon_to_target().
    
       a. If the argument is non-zero, virtio_balloon_to_target() executes
          its balloon half, which doesn't use the callback in any way.
    
          Odd, but works.
    
       b. If the argument is zero, virtio_balloon_to_target() executes its
          balloon stats half, just like in 1.  It either calls the callback
          right away, or arranges for it to be called later.
    
          Thus, the callback runs twice: use after free and double free.
    
    Test case: start with -S -device virtio-balloon, execute "balloon 0" in
    human monitor.  Runs the callback first from virtio_balloon_to_target(),
    then again from do_balloon().
    
    Reported-by: default avatarMike Cao <bcao@redhat.com>
    Signed-off-by: default avatarAmit Shah <amit.shah@redhat.com>
    Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
balloon.c 4.34 KiB