diff mbox

[dpdk-dev] eal: fix crash on mmap error in rte_eal_hugepage_attach()

Message ID 1475059977-27370-1-git-send-email-maciej.czekaj@caviumnetworks.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Maciej Czekaj Sept. 28, 2016, 10:52 a.m. UTC 
  From: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>

In ASLR-enabled system, it is possible that selected
virtual space is occupied by program segments. Therefore,
error path should not blindly unmap all memmory segments
but only those already mapped.

Steps that lead to crash:
1. memeseg 0 in secondary process overlaps
   with libc.so
2. mmap of /dev/zero fails for virtual space of memseg 0
3. munmap of memseg 0 leads to unmapping libc.so itself
4. app gets SIGSEGV after returning from syscall to libc

Fixes: ea329d7f8e34 ("mem: fix leak after mapping failure")

Signed-off-by: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
---
 lib/librte_eal/linuxapp/eal/eal_memory.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Sergio Gonzalez Monroy Oct. 3, 2016, 1:04 p.m. UTC | #1 
On 28/09/2016 11:52, maciej.czekaj@caviumnetworks.com wrote:
> From: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
>
> In ASLR-enabled system, it is possible that selected
> virtual space is occupied by program segments. Therefore,
> error path should not blindly unmap all memmory segments
> but only those already mapped.
>
> Steps that lead to crash:
> 1. memeseg 0 in secondary process overlaps
>     with libc.so
> 2. mmap of /dev/zero fails for virtual space of memseg 0
> 3. munmap of memseg 0 leads to unmapping libc.so itself
> 4. app gets SIGSEGV after returning from syscall to libc
>
> Fixes: ea329d7f8e34 ("mem: fix leak after mapping failure")
>
> Signed-off-by: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> ---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)

Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Thomas Monjalon Oct. 3, 2016, 2:06 p.m. UTC | #2 
2016-10-03 14:04, Sergio Gonzalez Monroy:
> On 28/09/2016 11:52, maciej.czekaj@caviumnetworks.com wrote:
> > From: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> >
> > In ASLR-enabled system, it is possible that selected
> > virtual space is occupied by program segments. Therefore,
> > error path should not blindly unmap all memmory segments
> > but only those already mapped.
> >
> > Steps that lead to crash:
> > 1. memeseg 0 in secondary process overlaps
> >     with libc.so
> > 2. mmap of /dev/zero fails for virtual space of memseg 0
> > 3. munmap of memseg 0 leads to unmapping libc.so itself
> > 4. app gets SIGSEGV after returning from syscall to libc
> >
> > Fixes: ea329d7f8e34 ("mem: fix leak after mapping failure")
> >
> > Signed-off-by: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> > ---
> >   lib/librte_eal/linuxapp/eal/eal_memory.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 612626c..1dfe223 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1545,6 +1545,7 @@  rte_eal_hugepage_attach(void)
 	struct hugepage_file *hp = NULL;
 	unsigned num_hp = 0;
 	unsigned i, s = 0; /* s used to track the segment number */
+	unsigned max_seg = RTE_MAX_MEMSEG;
 	off_t size;
 	int fd, fd_zero = -1, fd_hugepage = -1;
 
@@ -1603,6 +1604,9 @@  rte_eal_hugepage_attach(void)
 				"in /dev/zero to requested address [%p]: '%s'\n",
 				(unsigned long long)mcfg->memseg[s].len,
 				mcfg->memseg[s].addr, strerror(errno));
+			max_seg = s;
+			if (base_addr != MAP_FAILED)
+				munmap(base_addr, mcfg->memseg[s].len);
 			if (aslr_enabled() > 0) {
 				RTE_LOG(ERR, EAL, "It is recommended to "
 					"disable ASLR in the kernel "
@@ -1675,11 +1679,8 @@  rte_eal_hugepage_attach(void)
 	return 0;
 
 error:
-	s = 0;
-	while (s < RTE_MAX_MEMSEG && mcfg->memseg[s].len > 0) {
-		munmap(mcfg->memseg[s].addr, mcfg->memseg[s].len);
-		s++;
-	}
+	for (i = 0; i < max_seg && mcfg->memseg[i].len > 0; i++)
+		munmap(mcfg->memseg[i].addr, mcfg->memseg[i].len);
 	if (hp != NULL && hp != MAP_FAILED)
 		munmap(hp, size);
 	if (fd_zero >= 0)