patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0
@ 2020-01-16 10:44 Maxime Coquelin
  2020-01-17  7:51 ` Tiwei Bie
  2020-02-05  9:49 ` Maxime Coquelin
  0 siblings, 2 replies; 3+ messages in thread
From: Maxime Coquelin @ 2020-01-16 10:44 UTC (permalink / raw)
  To: dev, tiwei.bie, zhihong.wang; +Cc: Maxime Coquelin, stable, Ilja Van Sprundel

This patch catches an overflow that could happen if an
invalid region size or page alignement is provided by the
guest via the VHOST_USER_SET_MEM_TABLE request.

If the sum of the size to mmap and the alignment overflows
uint64_t, then RTE_ALIGN_CEIL(mmap_size, alignment) macro
will return 0. This value was passed as is as size argument
to mmap().

While kernel handling of mmap() syscall returns an error
if size is 0, it is better to catch it earlier and provide
a meaningful error log.

Fixes: ec09c280b839 ("vhost: fix mmap not aligned with hugepage size")
Cc: stable@dpdk.org

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 0b7d1e288e..41ec069cb6 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1145,6 +1145,21 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
 			goto err_mmap;
 		}
 		mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
+		if (mmap_size == 0) {
+			/*
+			 * It could happen if initial mmap_size + alignment
+			 * overflows the sizeof uint64, which could happen if
+			 * either mmap_size or alignment value is wrong.
+			 *
+			 * mmap() kernel implementation would return an error,
+			 * but better catch it before and provide useful info
+			 * in the logs.
+			 */
+			VHOST_LOG_CONFIG(ERR, "mmap size (0x%" PRIx64 ") "
+					"or alignment (0x%" PRIx64 ") is invalid\n",
+					reg->size + mmap_offset, alignment);
+			goto err_mmap;
+		}
 
 		populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
 		mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
-- 
2.21.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0
  2020-01-16 10:44 [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0 Maxime Coquelin
@ 2020-01-17  7:51 ` Tiwei Bie
  2020-02-05  9:49 ` Maxime Coquelin
  1 sibling, 0 replies; 3+ messages in thread
From: Tiwei Bie @ 2020-01-17  7:51 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, zhihong.wang, stable, Ilja Van Sprundel

On Thu, Jan 16, 2020 at 11:44:27AM +0100, Maxime Coquelin wrote:
> This patch catches an overflow that could happen if an
> invalid region size or page alignement is provided by the

s/alignement/alignment/

> guest via the VHOST_USER_SET_MEM_TABLE request.
> 
> If the sum of the size to mmap and the alignment overflows
> uint64_t, then RTE_ALIGN_CEIL(mmap_size, alignment) macro
> will return 0. This value was passed as is as size argument
> to mmap().
> 
> While kernel handling of mmap() syscall returns an error
> if size is 0, it is better to catch it earlier and provide
> a meaningful error log.
> 
> Fixes: ec09c280b839 ("vhost: fix mmap not aligned with hugepage size")
> Cc: stable@dpdk.org
> 
> Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost_user.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 0b7d1e288e..41ec069cb6 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1145,6 +1145,21 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  			goto err_mmap;
>  		}
>  		mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
> +		if (mmap_size == 0) {
> +			/*
> +			 * It could happen if initial mmap_size + alignment
> +			 * overflows the sizeof uint64, which could happen if
> +			 * either mmap_size or alignment value is wrong.
> +			 *
> +			 * mmap() kernel implementation would return an error,
> +			 * but better catch it before and provide useful info
> +			 * in the logs.
> +			 */
> +			VHOST_LOG_CONFIG(ERR, "mmap size (0x%" PRIx64 ") "
> +					"or alignment (0x%" PRIx64 ") is invalid\n",
> +					reg->size + mmap_offset, alignment);
> +			goto err_mmap;
> +		}
>  
>  		populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
>  		mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> -- 
> 2.21.0

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0
  2020-01-16 10:44 [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0 Maxime Coquelin
  2020-01-17  7:51 ` Tiwei Bie
@ 2020-02-05  9:49 ` Maxime Coquelin
  1 sibling, 0 replies; 3+ messages in thread
From: Maxime Coquelin @ 2020-02-05  9:49 UTC (permalink / raw)
  To: dev, tiwei.bie, zhihong.wang; +Cc: stable, Ilja Van Sprundel



On 1/16/20 11:44 AM, Maxime Coquelin wrote:
> This patch catches an overflow that could happen if an
> invalid region size or page alignement is provided by the
> guest via the VHOST_USER_SET_MEM_TABLE request.
> 
> If the sum of the size to mmap and the alignment overflows
> uint64_t, then RTE_ALIGN_CEIL(mmap_size, alignment) macro
> will return 0. This value was passed as is as size argument
> to mmap().
> 
> While kernel handling of mmap() syscall returns an error
> if size is 0, it is better to catch it earlier and provide
> a meaningful error log.
> 
> Fixes: ec09c280b839 ("vhost: fix mmap not aligned with hugepage size")
> Cc: stable@dpdk.org
> 
> Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost_user.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Applied to dpdk-next-virtio tree.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-02-05  9:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 10:44 [dpdk-stable] [PATCH] vhost: catch overflow causing mmap of size 0 Maxime Coquelin
2020-01-17  7:51 ` Tiwei Bie
2020-02-05  9:49 ` Maxime Coquelin

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git