[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 19/22] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PULL 19/22] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT |
Date: |
Fri, 8 Mar 2019 17:14:50 +0000 |
On Wed, 6 Mar 2019 at 11:55, Dr. David Alan Gilbert (git)
<address@hidden> wrote:
>
> From: Wei Wang <address@hidden>
>
> The new feature enables the virtio-balloon device to receive hints of
> guest free pages from the free page vq.
>
> A notifier is registered to the migration precopy notifier chain. The
> notifier calls free_page_start after the migration thread syncs the dirty
> bitmap, so that the free page optimization starts to clear bits of free
> pages from the bitmap. It calls the free_page_stop before the migration
> thread syncs the bitmap, which is the end of the current round of ram
> save. The free_page_stop is also called to stop the optimization in the
> case when there is an error occurred in the process of ram saving.
>
> Note: balloon will report pages which were free at the time of this call.
> As the reporting happens asynchronously, dirty bit logging must be
> enabled before this free_page_start call is made. Guest reporting must be
> disabled before the migration dirty bitmap is synchronized.
>
> Signed-off-by: Wei Wang <address@hidden>
> CC: Michael S. Tsirkin <address@hidden>
> CC: Dr. David Alan Gilbert <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: Peter Xu <address@hidden>
> Message-Id: <address@hidden>
> Reviewed-by: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> dgilbert: Dropped kernel header update, fixed up CMD_ID_* name change
> ---
Hi -- Coverity points out a use-after-free here (CID 1399412):
> +static bool get_free_page_hints(VirtIOBalloon *dev)
> +{
> + VirtQueueElement *elem;
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VirtQueue *vq = dev->free_page_vq;
> +
> + while (dev->block_iothread) {
> + qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
> + }
> +
> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> + if (!elem) {
> + return false;
> + }
> +
> + if (elem->out_num) {
> + uint32_t id;
> + size_t size = iov_to_buf(elem->out_sg, elem->out_num, 0,
> + &id, sizeof(id));
> + virtqueue_push(vq, elem, size);
> + g_free(elem);
Here we free elem...
> +
> + virtio_tswap32s(vdev, &id);
> + if (unlikely(size != sizeof(id))) {
> + virtio_error(vdev, "received an incorrect cmd id");
> + return false;
> + }
> + if (id == dev->free_page_report_cmd_id) {
> + dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
> + } else {
> + /*
> + * Stop the optimization only when it has started. This
> + * avoids a stale stop sign for the previous command.
> + */
> + if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
> + dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
> + }
> + }
> + }
> +
> + if (elem->in_num) {
...but we can fall through here and try to dereference elem...
> + if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
> + qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
> + elem->in_sg[0].iov_len);
> + }
> + virtqueue_push(vq, elem, 1);
> + g_free(elem);
...and then free it again.
> + }
> +
> + return true;
> +}
thanks
-- PMM
- [Qemu-devel] [PULL 10/22] tests: Add migration xbzrle test, (continued)
- [Qemu-devel] [PULL 10/22] tests: Add migration xbzrle test, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 11/22] migration: Create socket-address parameter, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 12/22] tests: Add basic migration precopy tcp test, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 13/22] bitmap: fix bitmap_count_one, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 15/22] migration: use bitmap_mutex in migration_bitmap_clear_dirty, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 16/22] migration: API to clear bits of guest free pages from the dirty bitmap, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 14/22] bitmap: bitmap_count_one_with_offset, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 17/22] migration/ram.c: add a notifier chain for precopy, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 18/22] migration/ram.c: add the free page optimization enable flag, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 19/22] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT, Dr. David Alan Gilbert (git), 2019/03/06
- Re: [Qemu-devel] [PULL 19/22] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT,
Peter Maydell <=
- [Qemu-devel] [PULL 20/22] Migration/colo.c: Fix double close bug when occur COLO failover, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 21/22] Migration/colo.c: Make COLO node running after failover, Dr. David Alan Gilbert (git), 2019/03/06
- [Qemu-devel] [PULL 22/22] qapi/migration.json: Remove a variable that doesn't exist in example, Dr. David Alan Gilbert (git), 2019/03/06
- Re: [Qemu-devel] [PULL 00/22] migration queue, no-reply, 2019/03/06
- Re: [Qemu-devel] [PULL 00/22] migration queue, Peter Maydell, 2019/03/06