On Tue, Apr 09, 2013 at 08:46:42AM -0700, Nicholas A. Bellinger wrote: > On Tue, 2013-04-09 at 17:16 +0800, Asias He wrote: > > If we fail to submit the allocated tv_vmd to tcm_vhost_submission_work, > > we will leak the tv_vmd. Free tv_vmd on fail path. > > > > Signed-off-by: Asias He <[email protected]> > > --- > > drivers/vhost/tcm_vhost.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c > > index 3351ed3..1f9116c 100644 > > --- a/drivers/vhost/tcm_vhost.c > > +++ b/drivers/vhost/tcm_vhost.c > > @@ -860,7 +860,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs, > > vq_err(vq, "Expecting virtio_scsi_cmd_resp, got %zu" > > " bytes, out: %d, in: %d\n", > > vq->iov[out].iov_len, out, in); > > - break; > > + goto err; > > } > > > > tv_cmd->tvc_resp = vq->iov[out].iov_base; > > @@ -882,7 +882,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs, > > " exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n", > > scsi_command_size(tv_cmd->tvc_cdb), > > TCM_VHOST_MAX_CDB_SIZE); > > - break; /* TODO */ > > + goto err; > > } > > tv_cmd->tvc_lun = ((v_req.lun[2] << 8) | v_req.lun[3]) & 0x3FFF; > > > > @@ -895,7 +895,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs, > > data_direction == DMA_TO_DEVICE); > > if (unlikely(ret)) { > > vq_err(vq, "Failed to map iov to sgl\n"); > > - break; /* TODO */ > > + goto err; > > } > > } > > > > Mmmm, I think these cases also require a VIRTIO_SCSI_S_BAD_TARGET + > __copy_to_user + vhost_add_used_and_signal similar to how !tv_tpg is > handled.. Otherwise virtio-scsi will end up in scsi timeout -> abort, > no..? > > Ditto for the vhost_scsi_allocate_cmd failure case..
Sent out new patches. > vhost-net uses vhost_discard_vq_desc for some failure cases, is that > needed here for the failure cases before __copy_from_user is called..? I don't think it is useful. vhost_discard_vq_desc reverse the effect of vhost_get_vq_desc. If we put it back in the queue and next time we will still fail. > > @@ -916,6 +916,11 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs, > > } > > > > mutex_unlock(&vq->mutex); > > + return; > > + > > +err: > > + vhost_scsi_free_cmd(tv_cmd); > > + mutex_unlock(&vq->mutex); > > } > > > > static void vhost_scsi_ctl_handle_kick(struct vhost_work *work) > > -- Asias _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
