diff mbox series

vhost: fix double read of descriptor flags

Message ID 20181205150926.4895-1-i.maximets@samsung.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix double read of descriptor flags |

Checks

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

Commit Message

Ilya Maximets Dec. 5, 2018, 3:09 p.m. UTC 
  Flags could be updated in a separate process leading to the
inconsistent check.

Additionally, read marked as 'volatile' to highlight the shared
nature of the variable and avoid such issues in the future.

Fixes: d3211c98c456 ("vhost: add helpers for packed virtqueues")
Cc: stable@dpdk.org

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/librte_vhost/vhost.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Maxime Coquelin Dec. 11, 2018, 2:47 p.m. UTC | #1 
On 12/5/18 4:09 PM, Ilya Maximets wrote:
> Flags could be updated in a separate process leading to the
> inconsistent check.
> 
> Additionally, read marked as 'volatile' to highlight the shared
> nature of the variable and avoid such issues in the future.
> 
> Fixes: d3211c98c456 ("vhost: add helpers for packed virtqueues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/librte_vhost/vhost.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 5218f1b12..84cbee2b6 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -393,8 +393,10 @@ vq_is_packed(struct virtio_net *dev)
>   static inline bool
>   desc_is_avail(struct vring_packed_desc *desc, bool wrap_counter)
>   {
> -	return wrap_counter == !!(desc->flags & VRING_DESC_F_AVAIL) &&
> -		wrap_counter != !!(desc->flags & VRING_DESC_F_USED);
> +	uint16_t flags = *((volatile uint16_t *) &desc->flags);
> +
> +	return wrap_counter == !!(flags & VRING_DESC_F_AVAIL) &&
> +		wrap_counter != !!(flags & VRING_DESC_F_USED);
>   }
>   
>   #define VHOST_LOG_PAGE	4096
> 

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

Thanks,
Maxime
Maxime Coquelin Dec. 11, 2018, 6:33 p.m. UTC | #2 
On 12/5/18 4:09 PM, Ilya Maximets wrote:
> Flags could be updated in a separate process leading to the
> inconsistent check.
> 
> Additionally, read marked as 'volatile' to highlight the shared
> nature of the variable and avoid such issues in the future.
> 
> Fixes: d3211c98c456 ("vhost: add helpers for packed virtqueues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/librte_vhost/vhost.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 5218f1b12..84cbee2b6 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -393,8 +393,10 @@ vq_is_packed(struct virtio_net *dev)
>   static inline bool
>   desc_is_avail(struct vring_packed_desc *desc, bool wrap_counter)
>   {
> -	return wrap_counter == !!(desc->flags & VRING_DESC_F_AVAIL) &&
> -		wrap_counter != !!(desc->flags & VRING_DESC_F_USED);
> +	uint16_t flags = *((volatile uint16_t *) &desc->flags);
> +
> +	return wrap_counter == !!(flags & VRING_DESC_F_AVAIL) &&
> +		wrap_counter != !!(flags & VRING_DESC_F_USED);
>   }
>   
>   #define VHOST_LOG_PAGE	4096
> 

Applied to dpdk-next-virtio.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 5218f1b12..84cbee2b6 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -393,8 +393,10 @@  vq_is_packed(struct virtio_net *dev)
 static inline bool
 desc_is_avail(struct vring_packed_desc *desc, bool wrap_counter)
 {
-	return wrap_counter == !!(desc->flags & VRING_DESC_F_AVAIL) &&
-		wrap_counter != !!(desc->flags & VRING_DESC_F_USED);
+	uint16_t flags = *((volatile uint16_t *) &desc->flags);
+
+	return wrap_counter == !!(flags & VRING_DESC_F_AVAIL) &&
+		wrap_counter != !!(flags & VRING_DESC_F_USED);
 }
 
 #define VHOST_LOG_PAGE	4096