Re: [Qemu-devel] [PATCH 08/10] iscsi: Fix NULL dereferences / races between task completion and abort

[Paolo Bonzini <pbonzini@redhat.com>]

Il 15/08/2012 15:56, Kevin Wolf ha scritto:
> @@ -76,6 +76,10 @@ static void
>  iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
>                      void *private_data)
>  {
> +    IscsiAIOCB *acb = (IscsiAIOCB *)private_data;
> +
> +    scsi_free_scsi_task(acb->task);
> +    acb->task = NULL;
>  }
>  
>  static void
> @@ -84,15 +88,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
>      IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
>      IscsiLun *iscsilun = acb->iscsilun;
>  
> -    acb->common.cb(acb->common.opaque, -ECANCELED);
>      acb->canceled = 1;
>  
> -    /* send a task mgmt call to the target to cancel the task on the target */
> -    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> -                                     iscsi_abort_task_cb, NULL);
> +    acb->common.cb(acb->common.opaque, -ECANCELED);
>  
> -    /* then also cancel the task locally in libiscsi */
> -    iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
> +    /* send a task mgmt call to the target to cancel the task on the target
> +     * this also cancels the task in libiscsi
> +     */
> +    iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> +                                     iscsi_abort_task_cb, &acb);

This is definitely a step forward, but iscsi_aio_cancel should actually
be synchronous.  Otherwise the target could write the data between the
callback and the sending of the TMF.  I'll send a patch for testing on
Monday.

Paolo