diff mbox series

[v3,1/3] net/virtio: add missing barrier before reading the flags

Message ID 20190109145015.3010-2-i.maximets@samsung.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series Missing barriers and VIRTIO_F_ORDER_PLATFORM. |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Ilya Maximets Jan. 9, 2019, 2:50 p.m. UTC 
  Reading the used->flags could be reordered with avail->idx update.
vhost in kernel disables notifications for the time of packets
receiving, like this:

    1. disable notify
    2. process packets
    3. enable notify
    4. has more packets ? goto 1

In case of reordering, virtio driver could read the flags on
step 2 while notifications disabled and update avail->idx after
the step 4, i.e. vhost will exit the loop on step 4 with
notifications enabled, but virtio will not notify.

Fixes: c1f86306a026 ("virtio: add new driver")
CC: stable@dpdk.org

Reported-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 drivers/net/virtio/virtqueue.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Maxime Coquelin Jan. 10, 2019, 2:31 p.m. UTC | #1 
On 1/9/19 3:50 PM, Ilya Maximets wrote:
> Reading the used->flags could be reordered with avail->idx update.
> vhost in kernel disables notifications for the time of packets
> receiving, like this:
> 
>      1. disable notify
>      2. process packets
>      3. enable notify
>      4. has more packets ? goto 1
> 
> In case of reordering, virtio driver could read the flags on
> step 2 while notifications disabled and update avail->idx after
> the step 4, i.e. vhost will exit the loop on step 4 with
> notifications enabled, but virtio will not notify.
> 
> Fixes: c1f86306a026 ("virtio: add new driver")
> CC: stable@dpdk.org
> 
> Reported-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   drivers/net/virtio/virtqueue.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index d8ae5cdec..dffa03669 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -415,6 +415,11 @@  vq_update_avail_ring(struct virtqueue *vq, uint16_t desc_idx)
 static inline int
 virtqueue_kick_prepare(struct virtqueue *vq)
 {
+	/*
+	 * Ensure updated avail->idx is visible to vhost before reading
+	 * the used->flags.
+	 */
+	virtio_mb();
 	return !(vq->vq_ring.used->flags & VRING_USED_F_NO_NOTIFY);
 }
 
@@ -423,6 +428,9 @@  virtqueue_kick_prepare_packed(struct virtqueue *vq)
 {
 	uint16_t flags;
 
+	/*
+	 * Ensure updated data is visible to vhost before reading the flags.
+	 */
 	virtio_mb();
 	flags = vq->ring_packed.device_event->desc_event_flags;