qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH 3/3] block/iscsi: fix ioctl cancel


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 3/3] block/iscsi: fix ioctl cancel use-after-free
Date: Fri, 2 Feb 2018 22:59:44 +0100

On Fri, Feb 2, 2018 at 10:16 PM, Stefan Hajnoczi <address@hidden> wrote:
> The ioctl request cancellation code assumes that requests do not
> complete once TASK ABORT has been sent to the iSCSI target.  The request
> completion callback is unconditionally invoked when TASK ABORT finishes.
> Therefore the request completion callback is invoked twice if the
> request does happen to complete before TASK ABORT.
>
> Futhermore, iscsi_aio_cancel() does not increment the request's
> reference count, causing a use-after-free when TASK ABORT finishes after
> the request has already completed.
>
> The iscsilun->mutex protection is also missing in iscsi_aio_cancel().
>
> This patch rewrites iscsi_aio_cancel() and iscsi_abort_task_cb() to
> avoid double completion, use-after-free, and to take iscsilun->mutex
> when needed.
>
> Reported-by: Felipe Franciosi <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  block/iscsi.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 1cfe1c647c..4566902d43 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -282,14 +282,19 @@ static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, 
> struct IscsiTask *iTask)
>      };
>  }
>
> +/* Called (via iscsi_service) with QemuMutex held. */
>  static void
>  iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void 
> *command_data,
>                      void *private_data)
>  {
>      IscsiAIOCB *acb = private_data;
>
> -    acb->status = -ECANCELED;
> -    iscsi_schedule_bh(acb);
> +    /* Skip if the request already completed */
> +    if (acb->status == -ECANCELED) {

There is a bug here.  acb->status can be -ECANCELED if the request
completed with COMMAND ABORTED.

I'll send a v2 of this patch tomorrow to solve this.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]