diff mbox

[dpdk-dev,v2,1/3] vhost: fix malloc in rte_vhost_get_mem_table

Message ID 1494500208-78788-1-git-send-email-dariuszx.stojaczyk@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stojaczyk, DariuszX May 11, 2017, 10:56 a.m. UTC 
  Amount of allocated memory was too small, causing buffer overflow.

Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
---
Removed Gerrit Change-Id
 lib/librte_vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jens Freimann May 11, 2017, 11:42 a.m. UTC | #1 
On Thu, May 11, 2017 at 12:56:46PM +0200, Dariusz Stojaczyk wrote:
> Amount of allocated memory was too small, causing buffer overflow.
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---
> Removed Gerrit Change-Id
>  lib/librte_vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index 0b19d2e..1f565fb 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -369,7 +369,7 @@ rte_vhost_get_mem_table(int vid, struct rte_vhost_memory **mem)
>  		return -1;
>  
>  	size = dev->mem->nregions * sizeof(struct rte_vhost_mem_region);
> -	m = malloc(size);
> +	m = malloc(sizeof(struct rte_vhost_memory) + size);

Why not just add it to the line above where size is calculated?
With that changed,

Reviewed-by: Jens Freimann <jfreimann@redhat.com> 


regards,
Jens
Stojaczyk, DariuszX May 11, 2017, 12:32 p.m. UTC | #2 
The size variable is still used a few lines later:
memcpy(m->regions, dev->mem->regions, size);
That line is ok. Only the amount of malloc'ed memory was too small.

-----Original Message-----
From: Jens Freimann [mailto:jfreiman@redhat.com] 
Sent: Thursday, May 11, 2017 1:42 PM
To: Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 1/3] vhost: fix malloc in rte_vhost_get_mem_table

On Thu, May 11, 2017 at 12:56:46PM +0200, Dariusz Stojaczyk wrote:
> Amount of allocated memory was too small, causing buffer overflow.
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---
> Removed Gerrit Change-Id
>  lib/librte_vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index 
> 0b19d2e..1f565fb 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -369,7 +369,7 @@ rte_vhost_get_mem_table(int vid, struct rte_vhost_memory **mem)
>  		return -1;
>  
>  	size = dev->mem->nregions * sizeof(struct rte_vhost_mem_region);
> -	m = malloc(size);
> +	m = malloc(sizeof(struct rte_vhost_memory) + size);

Why not just add it to the line above where size is calculated?
With that changed,

Reviewed-by: Jens Freimann <jfreimann@redhat.com> 


regards,
Jens

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
Stojaczyk, DariuszX May 11, 2017, 2:33 p.m. UTC | #3 
Fixes for memory-related undefined behavior issues in rte_vhost.

Daniel Verkamp (1):
  vhost: access VhostUsrMsg via packed struct

Dariusz Stojaczyk (2):
  vhost: fix malloc in rte_vhost_get_mem_table()
  vhost: free guest_pages in vhost_backend_cleanup()

 lib/librte_vhost/vhost.c      |  2 +-
 lib/librte_vhost/vhost_user.c | 64 ++++++++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 29 deletions(-)
Yuanhan Liu May 26, 2017, 8:40 a.m. UTC | #4 
On Fri, May 26, 2017 at 01:59:12PM +0200, Dariusz Stojaczyk wrote:
> Fixes for memory-related undefined behavior issues in rte_vhost.
> 
> Daniel Verkamp (1):
>   vhost: access VhostUsrMsg via packed struct
> 
> Dariusz Stojaczyk (2):
>   vhost: fix malloc in rte_vhost_get_mem_table()
>   vhost: free guest_pages in vhost_backend_cleanup()

Applied to dpdk-next-virtio.

Thanks!

	--yliu
> 
>  lib/librte_vhost/vhost.c      |  2 +-
>  lib/librte_vhost/vhost_user.c | 64 ++++++++++++++++++++++++-------------------
>  2 files changed, 37 insertions(+), 29 deletions(-)
> 
> -- 
> 2.7.4
Stojaczyk, DariuszX May 26, 2017, 11:59 a.m. UTC | #5 
Fixes for memory-related undefined behavior issues in rte_vhost.

Daniel Verkamp (1):
  vhost: access VhostUsrMsg via packed struct

Dariusz Stojaczyk (2):
  vhost: fix malloc in rte_vhost_get_mem_table()
  vhost: free guest_pages in vhost_backend_cleanup()

 lib/librte_vhost/vhost.c      |  2 +-
 lib/librte_vhost/vhost_user.c | 64 ++++++++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 29 deletions(-)
diff mbox

Patch

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 0b19d2e..1f565fb 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -369,7 +369,7 @@  rte_vhost_get_mem_table(int vid, struct rte_vhost_memory **mem)
 		return -1;
 
 	size = dev->mem->nregions * sizeof(struct rte_vhost_mem_region);
-	m = malloc(size);
+	m = malloc(sizeof(struct rte_vhost_memory) + size);
 	if (!m)
 		return -1;