Skip to content
  • Feng Lin's avatar
    c00d434a
    migration: fix the memory overwriting risk in add_to_iovec · c00d434a
    Feng Lin authored
    
    
    When testing migration, a Segmentation fault qemu core is generated.
    0  error_free (err=0x1)
    1  0x00007f8b862df647 in qemu_fclose (f=f@entry=0x55e06c247640)
    2  0x00007f8b8516d59a in migrate_fd_cleanup (s=s@entry=0x55e06c0e1ef0)
    3  0x00007f8b8516d66c in migrate_fd_cleanup_bh (opaque=0x55e06c0e1ef0)
    4  0x00007f8b8626a47f in aio_bh_poll (ctx=ctx@entry=0x55e06b5a16d0)
    5  0x00007f8b8626e71f in aio_dispatch (ctx=0x55e06b5a16d0)
    6  0x00007f8b8626a33d in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>)
    7  0x00007f8b866bdba4 in g_main_context_dispatch ()
    8  0x00007f8b8626cde9 in glib_pollfds_poll ()
    9  0x00007f8b8626ce62 in os_host_main_loop_wait (timeout=<optimized out>)
    10 0x00007f8b8626cffd in main_loop_wait (nonblocking=nonblocking@entry=0)
    11 0x00007f8b862ef01f in main_loop ()
    Using gdb print the struct QEMUFile f = {
      ...,
      iovcnt = 65, last_error = 21984,
      last_error_obj = 0x1, shutdown = true
    }
    Well iovcnt is overflow, because the max size of MAX_IOV_SIZE is 64.
    struct QEMUFile {
        ...;
        struct iovec iov[MAX_IOV_SIZE];
        unsigned int iovcnt;
        int last_error;
        Error *last_error_obj;
        bool shutdown;
    };
    iovcnt and last_error is overwrited by add_to_iovec().
    Right now, add_to_iovec() increase iovcnt before check the limit.
    And it seems that add_to_iovec() assumes that iovcnt will set to zero
    in qemu_fflush(). But qemu_fflush() will directly return when f->shutdown
    is true.
    
    The situation may occur when libvirtd restart during migration, after
    f->shutdown is set, before calling qemu_file_set_error() in
    qemu_file_shutdown().
    
    So the safiest way is checking the iovcnt before increasing it.
    
    Signed-off-by: default avatarFeng Lin <linfeng23@huawei.com>
    Message-Id: <20210625062138.1899-1-linfeng23@huawei.com>
    Reviewed-by: default avatarDr. David Alan Gilbert <dgilbert@redhat.com>
    Signed-off-by: default avatarDr. David Alan Gilbert <dgilbert@redhat.com>
      Fix typo in 'writeable' which is actually misnamed 'writable'
    c00d434a
    migration: fix the memory overwriting risk in add_to_iovec
    Feng Lin authored
    
    
    When testing migration, a Segmentation fault qemu core is generated.
    0  error_free (err=0x1)
    1  0x00007f8b862df647 in qemu_fclose (f=f@entry=0x55e06c247640)
    2  0x00007f8b8516d59a in migrate_fd_cleanup (s=s@entry=0x55e06c0e1ef0)
    3  0x00007f8b8516d66c in migrate_fd_cleanup_bh (opaque=0x55e06c0e1ef0)
    4  0x00007f8b8626a47f in aio_bh_poll (ctx=ctx@entry=0x55e06b5a16d0)
    5  0x00007f8b8626e71f in aio_dispatch (ctx=0x55e06b5a16d0)
    6  0x00007f8b8626a33d in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>)
    7  0x00007f8b866bdba4 in g_main_context_dispatch ()
    8  0x00007f8b8626cde9 in glib_pollfds_poll ()
    9  0x00007f8b8626ce62 in os_host_main_loop_wait (timeout=<optimized out>)
    10 0x00007f8b8626cffd in main_loop_wait (nonblocking=nonblocking@entry=0)
    11 0x00007f8b862ef01f in main_loop ()
    Using gdb print the struct QEMUFile f = {
      ...,
      iovcnt = 65, last_error = 21984,
      last_error_obj = 0x1, shutdown = true
    }
    Well iovcnt is overflow, because the max size of MAX_IOV_SIZE is 64.
    struct QEMUFile {
        ...;
        struct iovec iov[MAX_IOV_SIZE];
        unsigned int iovcnt;
        int last_error;
        Error *last_error_obj;
        bool shutdown;
    };
    iovcnt and last_error is overwrited by add_to_iovec().
    Right now, add_to_iovec() increase iovcnt before check the limit.
    And it seems that add_to_iovec() assumes that iovcnt will set to zero
    in qemu_fflush(). But qemu_fflush() will directly return when f->shutdown
    is true.
    
    The situation may occur when libvirtd restart during migration, after
    f->shutdown is set, before calling qemu_file_set_error() in
    qemu_file_shutdown().
    
    So the safiest way is checking the iovcnt before increasing it.
    
    Signed-off-by: default avatarFeng Lin <linfeng23@huawei.com>
    Message-Id: <20210625062138.1899-1-linfeng23@huawei.com>
    Reviewed-by: default avatarDr. David Alan Gilbert <dgilbert@redhat.com>
    Signed-off-by: default avatarDr. David Alan Gilbert <dgilbert@redhat.com>
      Fix typo in 'writeable' which is actually misnamed 'writable'
Loading