public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access
@ 2013-04-02 15:31 Asias He
  2013-04-02 15:39 ` Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: Asias He @ 2013-04-02 15:31 UTC (permalink / raw)
  To: Nicholas Bellinger
  Cc: Paolo Bonzini, Stefan Hajnoczi, Michael S. Tsirkin, Rusty Russell,
	kvm, virtualization, target-devel, Asias He

In vhost_scsi_handle_vq:

      tv_tpg = vs->vs_tpg[target];
      if (!tv_tpg) {
              ....
              return
      }

      tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req,

1) vs->vs_tpg[target] might change after the NULL check and 2) the above
line might access tv_tpg from vs->vs_tpg[target]. To prevent 2), use
ACCESS_ONCE. Thanks mst for catching this up!

Signed-off-by: Asias He <asias@redhat.com>
---
 drivers/vhost/tcm_vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 0524267..32d95e3 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -668,7 +668,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs,
 
 		/* Extract the tpgt */
 		target = v_req.lun[1];
-		tv_tpg = vs->vs_tpg[target];
+		tv_tpg = ACCESS_ONCE(vs->vs_tpg[target]);
 
 		/* Target does not exist, fail the request */
 		if (unlikely(!tv_tpg)) {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access
  2013-04-02 15:31 [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access Asias He
@ 2013-04-02 15:39 ` Michael S. Tsirkin
  2013-04-02 23:42   ` Nicholas A. Bellinger
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2013-04-02 15:39 UTC (permalink / raw)
  To: Asias He
  Cc: Nicholas Bellinger, Paolo Bonzini, Stefan Hajnoczi, Rusty Russell,
	kvm, virtualization, target-devel

On Tue, Apr 02, 2013 at 11:31:37PM +0800, Asias He wrote:
> In vhost_scsi_handle_vq:
> 
>       tv_tpg = vs->vs_tpg[target];
>       if (!tv_tpg) {
>               ....
>               return
>       }
> 
>       tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req,
> 
> 1) vs->vs_tpg[target] might change after the NULL check and 2) the above
> line might access tv_tpg from vs->vs_tpg[target]. To prevent 2), use
> ACCESS_ONCE. Thanks mst for catching this up!
> 
> Signed-off-by: Asias He <asias@redhat.com>

OK this might be ok for 3.9.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Nicholas can you pick this up pls?

For 3.10 I still think it's best to get rid of it
and stick vs->vs_tpg in vq->private_data.

> ---
>  drivers/vhost/tcm_vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> index 0524267..32d95e3 100644
> --- a/drivers/vhost/tcm_vhost.c
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -668,7 +668,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs,
>  
>  		/* Extract the tpgt */
>  		target = v_req.lun[1];
> -		tv_tpg = vs->vs_tpg[target];
> +		tv_tpg = ACCESS_ONCE(vs->vs_tpg[target]);
>  
>  		/* Target does not exist, fail the request */
>  		if (unlikely(!tv_tpg)) {
> -- 
> 1.8.1.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access
  2013-04-02 15:39 ` Michael S. Tsirkin
@ 2013-04-02 23:42   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 3+ messages in thread
From: Nicholas A. Bellinger @ 2013-04-02 23:42 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, virtualization, target-devel, Stefan Hajnoczi, Paolo Bonzini

On Tue, 2013-04-02 at 18:39 +0300, Michael S. Tsirkin wrote:
> On Tue, Apr 02, 2013 at 11:31:37PM +0800, Asias He wrote:
> > In vhost_scsi_handle_vq:
> > 
> >       tv_tpg = vs->vs_tpg[target];
> >       if (!tv_tpg) {
> >               ....
> >               return
> >       }
> > 
> >       tv_cmd = vhost_scsi_allocate_cmd(tv_tpg, &v_req,
> > 
> > 1) vs->vs_tpg[target] might change after the NULL check and 2) the above
> > line might access tv_tpg from vs->vs_tpg[target]. To prevent 2), use
> > ACCESS_ONCE. Thanks mst for catching this up!
> > 
> > Signed-off-by: Asias He <asias@redhat.com>
> 
> OK this might be ok for 3.9.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Nicholas can you pick this up pls?
> 

Applying to target-pending/master now.

> For 3.10 I still think it's best to get rid of it
> and stick vs->vs_tpg in vq->private_data.
> 

Your call here.  Given that vhost-scsi-pci code + Seabios w/ virtio-scsi
enabled will be broken without Asias's two extra vq->private_data and
initialize vq->last_used_idx changes on the list, they will certainly
need to hit 3.9.x code once your happy to ACK for v3.10.

Asias, I assume you'll be updating this soon..?

--nab

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-04-02 23:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-02 15:31 [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access Asias He
2013-04-02 15:39 ` Michael S. Tsirkin
2013-04-02 23:42   ` Nicholas A. Bellinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox