On Fri, Feb 2, 2018 at 10:16 PM, Stefan Hajnoczi <stefa...@redhat.com> 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 <fel...@nutanix.com>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> 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
