On Wed, 2012-04-18 at 15:46 +0200, Paolo Bonzini wrote:
> Fix a race in TMF path, where cmd may have been already freed
> by virtscsi_complete_free after waking up from the completion.
There's no may about this; the command will be freed long before the
completion waiter is awoken. The description could be clearer.
The problem is a use after free in virtscsi_tmf because the
virtio_scsi_command is freed before the completion returns.
The fix is to make callers specifying a completion responsible for
freeing the command in all cases.
James
> Cc: James Bottomley <JBottomley@xxxxxxxxxxxxx>
> Cc: linux-scsi@xxxxxxxxxxxxxxx
> Signed-off-by: Hu Tao <hutao@xxxxxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> drivers/scsi/virtio_scsi.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index efccd72..1b38431 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -175,7 +175,8 @@ static void virtscsi_complete_free(void *buf)
>
> if (cmd->comp)
> complete_all(cmd->comp);
> - mempool_free(cmd, virtscsi_cmd_pool);
> + else
> + mempool_free(cmd, virtscsi_cmd_pool);
> }
>
> static void virtscsi_ctrl_done(struct virtqueue *vq)
> @@ -311,21 +312,22 @@ out:
> static int virtscsi_tmf(struct virtio_scsi *vscsi, struct virtio_scsi_cmd *cmd)
> {
> DECLARE_COMPLETION_ONSTACK(comp);
> - int ret;
> + int ret = FAILED;
>
> cmd->comp = ∁
> - ret = virtscsi_kick_cmd(vscsi, vscsi->ctrl_vq, cmd,
> - sizeof cmd->req.tmf, sizeof cmd->resp.tmf,
> - GFP_NOIO);
> - if (ret < 0)
> - return FAILED;
> + if (virtscsi_kick_cmd(vscsi, vscsi->ctrl_vq, cmd,
> + sizeof cmd->req.tmf, sizeof cmd->resp.tmf,
> + GFP_NOIO) < 0)
> + goto out;
>
> wait_for_completion(&comp);
> - if (cmd->resp.tmf.response != VIRTIO_SCSI_S_OK &&
> - cmd->resp.tmf.response != VIRTIO_SCSI_S_FUNCTION_SUCCEEDED)
> - return FAILED;
> + if (cmd->resp.tmf.response == VIRTIO_SCSI_S_OK ||
> + cmd->resp.tmf.response == VIRTIO_SCSI_S_FUNCTION_SUCCEEDED)
> + ret = SUCCESS;
>
> - return SUCCESS;
> +out:
> + mempool_free(cmd, virtscsi_cmd_pool);
> + return ret;
> }
>
> static int virtscsi_device_reset(struct scsi_cmnd *sc)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
