[Qemu-devel] Re: [PATCH] virtio-blk: Fix use after free in error case

[Luiz Capitulino]

On Wed, 31 Mar 2010 17:46:59 +0200
Kevin Wolf <address@hidden> wrote:

> virtio_blk_req_complete frees the request, so we can't access it any more when
> calling bdrv_mon_event. Use the pointer that was copied earlier.

 Urgh, of course that I assume that by freeing 'req' we don't free 'req->dev'.

 To make this an oneliner we could just call bdrv_mon_event() before calling
virtio_blk_req_complete() (shouldn't matter for clients), but seems safer to
use 's' instead.

Acked-by: Luiz Capitulino <address@hidden>

> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  hw/virtio-blk.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 9915840..01d77b8 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -65,7 +65,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, 
> int error,
>      VirtIOBlock *s = req->dev;
>  
>      if (action == BLOCK_ERR_IGNORE) {
> -        bdrv_mon_event(req->dev->bs, BDRV_ACTION_IGNORE, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
>          return 0;
>      }
>  
> @@ -73,11 +73,11 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq 
> *req, int error,
>              || action == BLOCK_ERR_STOP_ANY) {
>          req->next = s->rq;
>          s->rq = req;
> -        bdrv_mon_event(req->dev->bs, BDRV_ACTION_STOP, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
>          vm_stop(0);
>      } else {
>          virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
> -        bdrv_mon_event(req->dev->bs, BDRV_ACTION_REPORT, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
>      }
>  
>      return 1;