patches for DPDK stable branches
 help / color / Atom feed
* [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind
@ 2020-01-06 20:55 Stephen Hemminger
  2020-01-07  8:43 ` David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Stephen Hemminger @ 2020-01-06 20:55 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

The loop to unwind existing mmaps was only unmapping the
first segment.

Also, remove obvious redundant assignment.

Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linux/eal/eal_memory.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/librte_eal/linux/eal/eal_memory.c b/lib/librte_eal/linux/eal/eal_memory.c
index 43e4ffc757bd..cf5b2433614b 100644
--- a/lib/librte_eal/linux/eal/eal_memory.c
+++ b/lib/librte_eal/linux/eal/eal_memory.c
@@ -1967,9 +1967,8 @@ eal_legacy_hugepage_attach(void)
 	close(fd);
 error:
 	/* map all segments into memory to make sure we get the addrs */
-	cur_seg = 0;
 	for (cur_seg = 0; cur_seg < i; cur_seg++) {
-		struct hugepage_file *hf = &hp[i];
+		struct hugepage_file *hf = &hp[cur_seg];
 		size_t map_sz = hf->size;
 		void *map_addr = hf->final_va;
 
-- 
2.20.1


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

* Re: [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind
  2020-01-06 20:55 [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind Stephen Hemminger
@ 2020-01-07  8:43 ` David Marchand
  2020-01-07 16:15 ` Burakov, Anatoly
  2020-01-22 17:06 ` [dpdk-stable] [PATCH v2] " Stephen Hemminger
  2 siblings, 0 replies; 5+ messages in thread
From: David Marchand @ 2020-01-07  8:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Burakov, Anatoly, dpdk stable

On Mon, Jan 6, 2020 at 9:56 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The loop to unwind existing mmaps was only unmapping the
> first segment.
>
> Also, remove obvious redundant assignment.
>
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/linux/eal/eal_memory.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/linux/eal/eal_memory.c b/lib/librte_eal/linux/eal/eal_memory.c
> index 43e4ffc757bd..cf5b2433614b 100644
> --- a/lib/librte_eal/linux/eal/eal_memory.c
> +++ b/lib/librte_eal/linux/eal/eal_memory.c
> @@ -1967,9 +1967,8 @@ eal_legacy_hugepage_attach(void)
>         close(fd);
>  error:
>         /* map all segments into memory to make sure we get the addrs */

This looks like a copy of the same loop from a few lines before in the
same function.
The comment can be removed.


> -       cur_seg = 0;
>         for (cur_seg = 0; cur_seg < i; cur_seg++) {
> -               struct hugepage_file *hf = &hp[i];
> +               struct hugepage_file *hf = &hp[cur_seg];
>                 size_t map_sz = hf->size;
>                 void *map_addr = hf->final_va;
>

map_addr and map_sz are unnecessary.

We should be safe with dereferencing hp if i != 0, but still how about:

error:
        if (hp != NULL && hp != MAP_FAILED) {
                unsigned int cur_seg;

                for (cur_seg = 0; cur_seg < i; cur_seg++)
                        munmap(hp[cur_seg].final_va, hp[cur_seg].size);
                munmap(hp, size);
        }
        if (fd_hugepage >= 0)
                close(fd_hugepage);
        return -1;


-- 
David Marchand


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

* Re: [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind
  2020-01-06 20:55 [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind Stephen Hemminger
  2020-01-07  8:43 ` David Marchand
@ 2020-01-07 16:15 ` Burakov, Anatoly
  2020-01-22 17:06 ` [dpdk-stable] [PATCH v2] " Stephen Hemminger
  2 siblings, 0 replies; 5+ messages in thread
From: Burakov, Anatoly @ 2020-01-07 16:15 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: stable

On 06-Jan-20 8:55 PM, Stephen Hemminger wrote:
> The loop to unwind existing mmaps was only unmapping the
> first segment.
> 
> Also, remove obvious redundant assignment.
> 
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* [dpdk-stable] [PATCH v2] mem: fix incorrect munmap in error unwind
  2020-01-06 20:55 [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind Stephen Hemminger
  2020-01-07  8:43 ` David Marchand
  2020-01-07 16:15 ` Burakov, Anatoly
@ 2020-01-22 17:06 ` " Stephen Hemminger
  2020-02-06 14:17   ` Thomas Monjalon
  2 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2020-01-22 17:06 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, anatoly.burakov, stable

The loop to unwind existing mmaps was only unmapping the
first segment and the error paths after mmap() were not
doing munmap of the current segment.

Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
v2 - incorporate feedback from David Marchand
     fix missing munmap of current segment

 lib/librte_eal/linux/eal/eal_memory.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/linux/eal/eal_memory.c b/lib/librte_eal/linux/eal/eal_memory.c
index 43e4ffc757bd..5604c2a7c04d 100644
--- a/lib/librte_eal/linux/eal/eal_memory.c
+++ b/lib/librte_eal/linux/eal/eal_memory.c
@@ -1928,7 +1928,7 @@ eal_legacy_hugepage_attach(void)
 		if (flock(fd, LOCK_SH) < 0) {
 			RTE_LOG(DEBUG, EAL, "%s(): Locking file failed: %s\n",
 				__func__, strerror(errno));
-			goto fd_error;
+			goto mmap_error;
 		}
 
 		/* find segment data */
@@ -1936,13 +1936,13 @@ eal_legacy_hugepage_attach(void)
 		if (msl == NULL) {
 			RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg list\n",
 				__func__);
-			goto fd_error;
+			goto mmap_error;
 		}
 		ms = rte_mem_virt2memseg(map_addr, msl);
 		if (ms == NULL) {
 			RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg\n",
 				__func__);
-			goto fd_error;
+			goto mmap_error;
 		}
 
 		msl_idx = msl - mcfg->memsegs;
@@ -1950,7 +1950,7 @@ eal_legacy_hugepage_attach(void)
 		if (ms_idx < 0) {
 			RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg idx\n",
 				__func__);
-			goto fd_error;
+			goto mmap_error;
 		}
 
 		/* store segment fd internally */
@@ -1963,18 +1963,15 @@ eal_legacy_hugepage_attach(void)
 	close(fd_hugepage);
 	return 0;
 
+mmap_error:
+	munmap(hp[i].final_va, hp[i].size);
 fd_error:
 	close(fd);
 error:
-	/* map all segments into memory to make sure we get the addrs */
-	cur_seg = 0;
-	for (cur_seg = 0; cur_seg < i; cur_seg++) {
-		struct hugepage_file *hf = &hp[i];
-		size_t map_sz = hf->size;
-		void *map_addr = hf->final_va;
+	/* unwind mmap's done so far */
+	for (cur_seg = 0; cur_seg < i; cur_seg++)
+		munmap(hp[cur_seg].final_va, hp[cur_seg].size);
 
-		munmap(map_addr, map_sz);
-	}
 	if (hp != NULL && hp != MAP_FAILED)
 		munmap(hp, size);
 	if (fd_hugepage >= 0)
-- 
2.20.1


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

* Re: [dpdk-stable] [PATCH v2] mem: fix incorrect munmap in error unwind
  2020-01-22 17:06 ` [dpdk-stable] [PATCH v2] " Stephen Hemminger
@ 2020-02-06 14:17   ` Thomas Monjalon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2020-02-06 14:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, stable, anatoly.burakov

22/01/2020 18:06, Stephen Hemminger:
> The loop to unwind existing mmaps was only unmapping the
> first segment and the error paths after mmap() were not
> doing munmap of the current segment.
> 
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> v2 - incorporate feedback from David Marchand
>      fix missing munmap of current segment

Applied, thanks




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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 20:55 [dpdk-stable] [PATCH] mem: fix incorrect munmap in error unwind Stephen Hemminger
2020-01-07  8:43 ` David Marchand
2020-01-07 16:15 ` Burakov, Anatoly
2020-01-22 17:06 ` [dpdk-stable] [PATCH v2] " Stephen Hemminger
2020-02-06 14:17   ` Thomas Monjalon

patches for DPDK stable branches

Archives are clonable:
	git clone --mirror http://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/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


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