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

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

[Kevin Wolf]

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.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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;
-- 
1.6.6.1

[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 <kwolf@redhat.com> 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 <lcapitulino@redhat.com>

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  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;