* [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
2013-04-02 15:39 ` Michael S. Tsirkin
0 siblings, 2 replies; 5+ 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] 5+ 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 15:39 ` Michael S. Tsirkin
1 sibling, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2013-04-02 15:39 UTC (permalink / raw)
To: Asias He
Cc: kvm, virtualization, target-devel, Stefan Hajnoczi, Paolo Bonzini
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] 5+ 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 15:39 ` Michael S. Tsirkin
2013-04-02 23:42 ` Nicholas A. Bellinger
1 sibling, 1 reply; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
* [PATCH] tcm_vhost: Use ACCESS_ONCE for vs->vs_tpg[target] access
@ 2013-04-02 15:31 Asias He
0 siblings, 0 replies; 5+ messages in thread
From: Asias He @ 2013-04-02 15:31 UTC (permalink / raw)
To: Nicholas Bellinger
Cc: kvm, Michael S. Tsirkin, virtualization, target-devel,
Stefan Hajnoczi, Paolo Bonzini
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] 5+ messages in thread
end of thread, other threads:[~2013-04-02 23:42 UTC | newest]
Thread overview: 5+ 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 15:39 ` Michael S. Tsirkin
2013-04-02 23:42 ` Nicholas A. Bellinger
-- strict thread matches above, loose matches on Subject: below --
2013-04-02 15:31 Asias He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.