[RFC,03/27] vhost: fix IOTLB entries overlap check with previous entry

  Commit 22b6d0ac691a ("vhost: fix madvise IOTLB entries pages overlap check")
fixed the check to ensure the entry to be removed does not
overlap with the next one in the IOTLB cache before marking
it as DONTDUMP with madvise(). This is not enough, because
the same issue is present when comparing with the previous
entry in the cache, where the end address of the previous
entry should be used, not the start one.

Fixes: dea092d0addb ("vhost: fix madvise arguments alignment")
Cc: stable@dpdk.org

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

On Fri, Mar 31, 2023 at 11:43 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Commit 22b6d0ac691a ("vhost: fix madvise IOTLB entries pages overlap check")
> fixed the check to ensure the entry to be removed does not
> overlap with the next one in the IOTLB cache before marking
> it as DONTDUMP with madvise(). This is not enough, because
> the same issue is present when comparing with the previous
> entry in the cache, where the end address of the previous
> entry should be used, not the start one.
>
> Fixes: dea092d0addb ("vhost: fix madvise arguments alignment")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Hi Maxime,

This makes sense.

Acked-by: Mike Pattrick <mkp@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 3f45bc6061..870c8acb88 100644
> --- a/lib/vhost/iotlb.c
> +++ b/lib/vhost/iotlb.c
> @@ -178,8 +178,8 @@ vhost_user_iotlb_cache_random_evict(struct virtio_net *dev, struct vhost_virtque
>                         mask = ~(alignment - 1);
>
>                         /* Don't disable coredump if the previous node is in the same page */
> -                       if (prev_node == NULL ||
> -                                       (node->uaddr & mask) != (prev_node->uaddr & mask)) {
> +                       if (prev_node == NULL || (node->uaddr & mask) !=
> +                                       ((prev_node->uaddr + prev_node->size - 1) & 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 + node->size - 1) & mask) !=
> @@ -283,8 +283,8 @@ vhost_user_iotlb_cache_remove(struct virtio_net *dev, struct vhost_virtqueue *vq
>                         mask = ~(alignment-1);
>
>                         /* Don't disable coredump if the previous node is in the same page */
> -                       if (prev_node == NULL ||
> -                                       (node->uaddr & mask) != (prev_node->uaddr & mask)) {
> +                       if (prev_node == NULL || (node->uaddr & mask) !=
> +                                       ((prev_node->uaddr + prev_node->size - 1) & 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 + node->size - 1) & mask) !=
> --
> 2.39.2
>
  

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, March 31, 2023 11:43 PM
> To: dev@dpdk.org; david.marchand@redhat.com; Xia, Chenbo
> <chenbo.xia@intel.com>; mkp@redhat.com; fbl@redhat.com;
> jasowang@redhat.com; Liang, Cunming <cunming.liang@intel.com>; Xie, Yongji
> <xieyongji@bytedance.com>; echaudro@redhat.com; eperezma@redhat.com;
> amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org
> Subject: [RFC 03/27] vhost: fix IOTLB entries overlap check with previous
> entry
> 
> Commit 22b6d0ac691a ("vhost: fix madvise IOTLB entries pages overlap
> check")
> fixed the check to ensure the entry to be removed does not
> overlap with the next one in the IOTLB cache before marking
> it as DONTDUMP with madvise(). This is not enough, because
> the same issue is present when comparing with the previous
> entry in the cache, where the end address of the previous
> entry should be used, not the start one.
> 
> Fixes: dea092d0addb ("vhost: fix madvise arguments alignment")
> Cc: stable@dpdk.org
> 
> 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 3f45bc6061..870c8acb88 100644
> --- a/lib/vhost/iotlb.c
> +++ b/lib/vhost/iotlb.c
> @@ -178,8 +178,8 @@ vhost_user_iotlb_cache_random_evict(struct virtio_net
> *dev, struct vhost_virtque
>  			mask = ~(alignment - 1);
> 
>  			/* Don't disable coredump if the previous node is in the
> same page */
> -			if (prev_node == NULL ||
> -					(node->uaddr & mask) != (prev_node->uaddr &
> mask)) {
> +			if (prev_node == NULL || (node->uaddr & mask) !=
> +					((prev_node->uaddr + prev_node->size - 1) &
> 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 + node-
> >size - 1) & mask) !=
> @@ -283,8 +283,8 @@ vhost_user_iotlb_cache_remove(struct virtio_net *dev,
> struct vhost_virtqueue *vq
>  			mask = ~(alignment-1);
> 
>  			/* Don't disable coredump if the previous node is in the
> same page */
> -			if (prev_node == NULL ||
> -					(node->uaddr & mask) != (prev_node->uaddr &
> mask)) {
> +			if (prev_node == NULL || (node->uaddr & mask) !=
> +					((prev_node->uaddr + prev_node->size - 1) &
> 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 + node-
> >size - 1) & mask) !=
> --
> 2.39.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>