vhost: fix madvise IOTLB entries pages overlap check

Message ID 20230315113158.442632-1-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix madvise IOTLB entries pages overlap check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional fail Functional issues

Commit Message

Maxime Coquelin March 15, 2023, 11:31 a.m. UTC
  At removal time, when testing whether the IOTLB entry has
shared pages with the previous and next entries in the
cache, it checks whether the start address of the entry to
be removed is on the same page as the start address of the
next entry in the cache.

This is not correct, as an entry could cover several page
so the end address of the entry to be remove should be
used. This patch address this issue.

Fixes: dea092d0addb ("vhost: fix madvise arguments alignment")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/iotlb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Maxime Coquelin March 15, 2023, 11:37 a.m. UTC | #1
On 3/15/23 12:31, Maxime Coquelin wrote:
> At removal time, when testing whether the IOTLB entry has
> shared pages with the previous and next entries in the
> cache, it checks whether the start address of the entry to
> be removed is on the same page as the start address of the
> next entry in the cache.
> 
> This is not correct, as an entry could cover several page
> so the end address of the entry to be remove should be
> used. This patch address this issue.
> 
> Fixes: dea092d0addb ("vhost: fix madvise arguments alignment")
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   lib/vhost/iotlb.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c
> index 11785392ac..bfa04a0439 100644
> --- a/lib/vhost/iotlb.c
> +++ b/lib/vhost/iotlb.c
> @@ -182,8 +182,8 @@ vhost_user_iotlb_cache_random_evict(struct virtio_net *dev, struct vhost_virtque
>   					(node->uaddr & mask) != (prev_node->uaddr & mask)) {
>   				next_node = RTE_TAILQ_NEXT(node, next);
>   				/* Don't disable coredump if the next node is in the same page */
> -				if (next_node == NULL ||
> -						(node->uaddr & mask) != (next_node->uaddr & mask))
> +				if (next_node == NULL || ((node->uaddr + node->size) & mask) !=

Oups, forgot to amend a change before generating the patch.
It should be (node->uaddr + node->size - 1) as we want the end address
of the entry.

> +						(next_node->uaddr & mask))
>   					mem_set_dump((void *)(uintptr_t)node->uaddr, node->size,
>   							false, alignment);
>   			}
> @@ -287,8 +287,8 @@ vhost_user_iotlb_cache_remove(struct virtio_net *dev, struct vhost_virtqueue *vq
>   					(node->uaddr & mask) != (prev_node->uaddr & mask)) {
>   				next_node = RTE_TAILQ_NEXT(node, next);
>   				/* Don't disable coredump if the next node is in the same page */
> -				if (next_node == NULL ||
> -						(node->uaddr & mask) != (next_node->uaddr & mask))
> +				if (next_node == NULL || ((node->uaddr + node->size) & mask) !=

Same here.

> +						(next_node->uaddr & mask))
>   					mem_set_dump((void *)(uintptr_t)node->uaddr, node->size,
>   							false, alignment);
>   			}

Sending v2.
  

Patch

diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c
index 11785392ac..bfa04a0439 100644
--- a/lib/vhost/iotlb.c
+++ b/lib/vhost/iotlb.c
@@ -182,8 +182,8 @@  vhost_user_iotlb_cache_random_evict(struct virtio_net *dev, struct vhost_virtque
 					(node->uaddr & mask) != (prev_node->uaddr & mask)) {
 				next_node = RTE_TAILQ_NEXT(node, next);
 				/* Don't disable coredump if the next node is in the same page */
-				if (next_node == NULL ||
-						(node->uaddr & mask) != (next_node->uaddr & mask))
+				if (next_node == NULL || ((node->uaddr + node->size) & mask) !=
+						(next_node->uaddr & mask))
 					mem_set_dump((void *)(uintptr_t)node->uaddr, node->size,
 							false, alignment);
 			}
@@ -287,8 +287,8 @@  vhost_user_iotlb_cache_remove(struct virtio_net *dev, struct vhost_virtqueue *vq
 					(node->uaddr & mask) != (prev_node->uaddr & mask)) {
 				next_node = RTE_TAILQ_NEXT(node, next);
 				/* Don't disable coredump if the next node is in the same page */
-				if (next_node == NULL ||
-						(node->uaddr & mask) != (next_node->uaddr & mask))
+				if (next_node == NULL || ((node->uaddr + node->size) & mask) !=
+						(next_node->uaddr & mask))
 					mem_set_dump((void *)(uintptr_t)node->uaddr, node->size,
 							false, alignment);
 			}