public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes
@ 2019-02-11 22:15 Jan Stancek
  2019-02-11 23:54 ` Daniel Mentz
  2019-02-19 13:29 ` Li Wang
  0 siblings, 2 replies; 5+ messages in thread
From: Jan Stancek @ 2019-02-11 22:15 UTC (permalink / raw)
  To: ltp

exec_func() is dummy/empty function. Try to align it so we don't
need to worry about copying 2 pages. But also check that compiler
aligned it and there's sufficient space between start of func_exec
and end of page.

This patch also removes copy_sz, which is now replaced with page_sz.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/mprotect/Makefile     |  2 +
 testcases/kernel/syscalls/mprotect/mprotect04.c | 55 +++++++++++--------------
 2 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/testcases/kernel/syscalls/mprotect/Makefile b/testcases/kernel/syscalls/mprotect/Makefile
index bd617d806675..bc5c8bc10395 100644
--- a/testcases/kernel/syscalls/mprotect/Makefile
+++ b/testcases/kernel/syscalls/mprotect/Makefile
@@ -20,4 +20,6 @@ top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
+mprotect04: CFLAGS += -falign-functions=64
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mprotect/mprotect04.c b/testcases/kernel/syscalls/mprotect/mprotect04.c
index 60941a4220d5..0f7890dca03b 100644
--- a/testcases/kernel/syscalls/mprotect/mprotect04.c
+++ b/testcases/kernel/syscalls/mprotect/mprotect04.c
@@ -55,7 +55,7 @@ int TST_TOTAL = ARRAY_SIZE(testfunc);
 
 static volatile int sig_caught;
 static sigjmp_buf env;
-static unsigned int copy_sz;
+static unsigned int page_sz;
 typedef void (*func_ptr_t)(void);
 
 int main(int ac, char **av)
@@ -88,7 +88,7 @@ static void setup(void)
 {
 	tst_tmpdir();
 	tst_sig(NOFORK, sighandler, cleanup);
-	copy_sz = getpagesize() * 2;
+	page_sz = getpagesize();
 
 	TEST_PAUSE;
 }
@@ -96,12 +96,9 @@ static void setup(void)
 static void testfunc_protnone(void)
 {
 	char *addr;
-	int page_sz;
 
 	sig_caught = 0;
 
-	page_sz = getpagesize();
-
 	addr = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
 					 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
@@ -133,7 +130,7 @@ static void testfunc_protnone(void)
 
 #ifdef __ia64__
 
-static char exec_func[] = {
+static char exec_func[] __attribute__ ((aligned (64))) = {
 	0x11, 0x00, 0x00, 0x00, 0x01, 0x00, /* nop.m 0x0             */
 	0x00, 0x00, 0x00, 0x02, 0x00, 0x80, /* nop.i 0x0             */
 	0x08, 0x00, 0x84, 0x00,             /* br.ret.sptk.many b0;; */
@@ -210,42 +207,33 @@ typedef struct {
  * Copy page where &exec_func resides. Also try to copy subsequent page
  * in case exec_func is close to page boundary.
  */
-static void *get_func(void *mem)
+static void *get_func(void *mem, uintptr_t *func_page_offset)
 {
 	uintptr_t page_sz = getpagesize();
 	uintptr_t page_mask = ~(page_sz - 1);
-	uintptr_t func_page_offset;
 	void *func_copy_start, *page_to_copy;
 	void *mem_start = mem;
 
 #ifdef USE_FUNCTION_DESCRIPTORS
 	func_descr_t *opd =  (func_descr_t *)&exec_func;
-	func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
-	func_copy_start = mem + func_page_offset;
+	*func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
+	func_copy_start = mem + *func_page_offset;
 	page_to_copy = (void *)((uintptr_t)opd->entry & page_mask);
 #else
-	func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
-	func_copy_start = mem + func_page_offset;
+	*func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
+	func_copy_start = mem + *func_page_offset;
 	page_to_copy = (void *)((uintptr_t)&exec_func & page_mask);
 #endif
+	tst_resm(TINFO, "exec_func: %p, page_to_copy: %p",
+		&exec_func, page_to_copy);
 
 	/* copy 1st page, if it's not present something is wrong */
-	if (!page_present(page_to_copy)) {
-		tst_resm(TINFO, "exec_func: %p, page_to_copy: %p\n",
-			&exec_func, page_to_copy);
+	if (!page_present(page_to_copy))
 		tst_brkm(TBROK, cleanup, "page_to_copy not present\n");
-	}
-	memcpy(mem, page_to_copy, page_sz);
 
-	/* copy 2nd page if possible */
-	mem += page_sz;
-	page_to_copy += page_sz;
-	if (page_present(page_to_copy))
-		memcpy(mem, page_to_copy, page_sz);
-	else
-		memset(mem, 0, page_sz);
+	memcpy(mem, page_to_copy, page_sz);
 
-	clear_cache(mem_start, copy_sz);
+	clear_cache(mem_start, page_sz);
 
 	/* return pointer to area where copy of exec_func resides */
 	return func_copy_start;
@@ -256,23 +244,30 @@ static void *get_func(void *mem)
 static void testfunc_protexec(void)
 {
 	func_ptr_t func;
+	uintptr_t func_page_offset;
 	void *p;
 
 	sig_caught = 0;
 
-	p = SAFE_MMAP(cleanup, 0, copy_sz, PROT_READ | PROT_WRITE,
+	p = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
 		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 #ifdef USE_FUNCTION_DESCRIPTORS
 	func_descr_t opd;
-	opd.entry = (uintptr_t)get_func(p);
+	opd.entry = (uintptr_t)get_func(p, &func_page_offset);
 	func = (func_ptr_t)&opd;
 #else
-	func = get_func(p);
+	func = get_func(p, &func_page_offset);
 #endif
 
+	if (func_page_offset + 64 >= page_sz) {
+		SAFE_MUNMAP(cleanup, p, page_sz);
+		tst_brkm(TCONF, cleanup, "func too close to page boundary, "
+			"maybe your compiler ignores -falign-functions?");
+	}
+
 	/* Change the protection to PROT_EXEC. */
-	TEST(mprotect(p, copy_sz, PROT_EXEC));
+	TEST(mprotect(p, page_sz, PROT_EXEC));
 
 	if (TEST_RETURN == -1) {
 		tst_resm(TFAIL | TTERRNO, "mprotect failed");
@@ -294,7 +289,7 @@ static void testfunc_protexec(void)
 		}
 	}
 
-	SAFE_MUNMAP(cleanup, p, copy_sz);
+	SAFE_MUNMAP(cleanup, p, page_sz);
 }
 
 static void cleanup(void)
-- 
1.8.3.1


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

* [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes
  2019-02-11 22:15 [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes Jan Stancek
@ 2019-02-11 23:54 ` Daniel Mentz
  2019-02-12  7:40   ` Jan Stancek
  2019-02-19 13:29 ` Li Wang
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Mentz @ 2019-02-11 23:54 UTC (permalink / raw)
  To: ltp

Two comments below. Otherwise, I'm fine with this. I reviewed this
patch and found it to be working on an aarch64 platform.

On Mon, Feb 11, 2019 at 2:15 PM Jan Stancek <jstancek@redhat.com> wrote:
>
> exec_func() is dummy/empty function. Try to align it so we don't
> need to worry about copying 2 pages. But also check that compiler
> aligned it and there's sufficient space between start of func_exec
> and end of page.
>
> This patch also removes copy_sz, which is now replaced with page_sz.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  testcases/kernel/syscalls/mprotect/Makefile     |  2 +
>  testcases/kernel/syscalls/mprotect/mprotect04.c | 55 +++++++++++--------------
>  2 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mprotect/Makefile b/testcases/kernel/syscalls/mprotect/Makefile
> index bd617d806675..bc5c8bc10395 100644
> --- a/testcases/kernel/syscalls/mprotect/Makefile
> +++ b/testcases/kernel/syscalls/mprotect/Makefile
> @@ -20,4 +20,6 @@ top_srcdir            ?= ../../../..
>
>  include $(top_srcdir)/include/mk/testcases.mk
>
> +mprotect04: CFLAGS += -falign-functions=64
> +
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/mprotect/mprotect04.c b/testcases/kernel/syscalls/mprotect/mprotect04.c
> index 60941a4220d5..0f7890dca03b 100644
> --- a/testcases/kernel/syscalls/mprotect/mprotect04.c
> +++ b/testcases/kernel/syscalls/mprotect/mprotect04.c
> @@ -55,7 +55,7 @@ int TST_TOTAL = ARRAY_SIZE(testfunc);
>
>  static volatile int sig_caught;
>  static sigjmp_buf env;
> -static unsigned int copy_sz;
> +static unsigned int page_sz;
>  typedef void (*func_ptr_t)(void);
>
>  int main(int ac, char **av)
> @@ -88,7 +88,7 @@ static void setup(void)
>  {
>         tst_tmpdir();
>         tst_sig(NOFORK, sighandler, cleanup);
> -       copy_sz = getpagesize() * 2;
> +       page_sz = getpagesize();
>
>         TEST_PAUSE;
>  }
> @@ -96,12 +96,9 @@ static void setup(void)
>  static void testfunc_protnone(void)
>  {
>         char *addr;
> -       int page_sz;
>
>         sig_caught = 0;
>
> -       page_sz = getpagesize();
> -
>         addr = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
>                                          MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> @@ -133,7 +130,7 @@ static void testfunc_protnone(void)
>
>  #ifdef __ia64__
>
> -static char exec_func[] = {
> +static char exec_func[] __attribute__ ((aligned (64))) = {
>         0x11, 0x00, 0x00, 0x00, 0x01, 0x00, /* nop.m 0x0             */
>         0x00, 0x00, 0x00, 0x02, 0x00, 0x80, /* nop.i 0x0             */
>         0x08, 0x00, 0x84, 0x00,             /* br.ret.sptk.many b0;; */
> @@ -210,42 +207,33 @@ typedef struct {
>   * Copy page where &exec_func resides. Also try to copy subsequent page
>   * in case exec_func is close to page boundary.
>   */
> -static void *get_func(void *mem)
> +static void *get_func(void *mem, uintptr_t *func_page_offset)
>  {
>         uintptr_t page_sz = getpagesize();
>         uintptr_t page_mask = ~(page_sz - 1);
> -       uintptr_t func_page_offset;
>         void *func_copy_start, *page_to_copy;
>         void *mem_start = mem;
>
>  #ifdef USE_FUNCTION_DESCRIPTORS
>         func_descr_t *opd =  (func_descr_t *)&exec_func;
> -       func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
> -       func_copy_start = mem + func_page_offset;
> +       *func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
> +       func_copy_start = mem + *func_page_offset;
>         page_to_copy = (void *)((uintptr_t)opd->entry & page_mask);
>  #else
> -       func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
> -       func_copy_start = mem + func_page_offset;
> +       *func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
> +       func_copy_start = mem + *func_page_offset;
>         page_to_copy = (void *)((uintptr_t)&exec_func & page_mask);
>  #endif
> +       tst_resm(TINFO, "exec_func: %p, page_to_copy: %p",
> +               &exec_func, page_to_copy);

This was previously inside the body of the if statement right below.
Not sure if it was intended to always print this information, but I'm
fine either way.

>
>         /* copy 1st page, if it's not present something is wrong */
> -       if (!page_present(page_to_copy)) {
> -               tst_resm(TINFO, "exec_func: %p, page_to_copy: %p\n",
> -                       &exec_func, page_to_copy);
> +       if (!page_present(page_to_copy))
>                 tst_brkm(TBROK, cleanup, "page_to_copy not present\n");
> -       }
> -       memcpy(mem, page_to_copy, page_sz);
>
> -       /* copy 2nd page if possible */
> -       mem += page_sz;
> -       page_to_copy += page_sz;
> -       if (page_present(page_to_copy))
> -               memcpy(mem, page_to_copy, page_sz);
> -       else
> -               memset(mem, 0, page_sz);
> +       memcpy(mem, page_to_copy, page_sz);
>
> -       clear_cache(mem_start, copy_sz);
> +       clear_cache(mem_start, page_sz);
>
>         /* return pointer to area where copy of exec_func resides */
>         return func_copy_start;
> @@ -256,23 +244,30 @@ static void *get_func(void *mem)
>  static void testfunc_protexec(void)
>  {
>         func_ptr_t func;
> +       uintptr_t func_page_offset;
>         void *p;
>
>         sig_caught = 0;
>
> -       p = SAFE_MMAP(cleanup, 0, copy_sz, PROT_READ | PROT_WRITE,
> +       p = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
>                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
>  #ifdef USE_FUNCTION_DESCRIPTORS
>         func_descr_t opd;
> -       opd.entry = (uintptr_t)get_func(p);
> +       opd.entry = (uintptr_t)get_func(p, &func_page_offset);
>         func = (func_ptr_t)&opd;
>  #else
> -       func = get_func(p);
> +       func = get_func(p, &func_page_offset);
>  #endif
>
> +       if (func_page_offset + 64 >= page_sz) {

I'm wondering if this should be ">" not ">=". If the compiler decides
to use the last 64 bytes in a page and locates the function at offset
0xfc0, then that's still ok: 0xfc0 + 0x40 == page_sz

> +               SAFE_MUNMAP(cleanup, p, page_sz);
> +               tst_brkm(TCONF, cleanup, "func too close to page boundary, "
> +                       "maybe your compiler ignores -falign-functions?");
> +       }
> +
>         /* Change the protection to PROT_EXEC. */
> -       TEST(mprotect(p, copy_sz, PROT_EXEC));
> +       TEST(mprotect(p, page_sz, PROT_EXEC));
>
>         if (TEST_RETURN == -1) {
>                 tst_resm(TFAIL | TTERRNO, "mprotect failed");
> @@ -294,7 +289,7 @@ static void testfunc_protexec(void)
>                 }
>         }
>
> -       SAFE_MUNMAP(cleanup, p, copy_sz);
> +       SAFE_MUNMAP(cleanup, p, page_sz);
>  }
>
>  static void cleanup(void)
> --
> 1.8.3.1
>

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

* [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes
  2019-02-11 23:54 ` Daniel Mentz
@ 2019-02-12  7:40   ` Jan Stancek
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Stancek @ 2019-02-12  7:40 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> Two comments below. Otherwise, I'm fine with this. I reviewed this
> patch and found it to be working on an aarch64 platform.
> 
> On Mon, Feb 11, 2019 at 2:15 PM Jan Stancek <jstancek@redhat.com> wrote:
> >
> > exec_func() is dummy/empty function. Try to align it so we don't
> > need to worry about copying 2 pages. But also check that compiler
> > aligned it and there's sufficient space between start of func_exec
> > and end of page.
> >
> > This patch also removes copy_sz, which is now replaced with page_sz.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  testcases/kernel/syscalls/mprotect/Makefile     |  2 +
> >  testcases/kernel/syscalls/mprotect/mprotect04.c | 55
> >  +++++++++++--------------
> >  2 files changed, 27 insertions(+), 30 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/mprotect/Makefile
> > b/testcases/kernel/syscalls/mprotect/Makefile
> > index bd617d806675..bc5c8bc10395 100644
> > --- a/testcases/kernel/syscalls/mprotect/Makefile
> > +++ b/testcases/kernel/syscalls/mprotect/Makefile
> > @@ -20,4 +20,6 @@ top_srcdir            ?= ../../../..
> >
> >  include $(top_srcdir)/include/mk/testcases.mk
> >
> > +mprotect04: CFLAGS += -falign-functions=64
> > +
> >  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/syscalls/mprotect/mprotect04.c
> > b/testcases/kernel/syscalls/mprotect/mprotect04.c
> > index 60941a4220d5..0f7890dca03b 100644
> > --- a/testcases/kernel/syscalls/mprotect/mprotect04.c
> > +++ b/testcases/kernel/syscalls/mprotect/mprotect04.c
> > @@ -55,7 +55,7 @@ int TST_TOTAL = ARRAY_SIZE(testfunc);
> >
> >  static volatile int sig_caught;
> >  static sigjmp_buf env;
> > -static unsigned int copy_sz;
> > +static unsigned int page_sz;
> >  typedef void (*func_ptr_t)(void);
> >
> >  int main(int ac, char **av)
> > @@ -88,7 +88,7 @@ static void setup(void)
> >  {
> >         tst_tmpdir();
> >         tst_sig(NOFORK, sighandler, cleanup);
> > -       copy_sz = getpagesize() * 2;
> > +       page_sz = getpagesize();
> >
> >         TEST_PAUSE;
> >  }
> > @@ -96,12 +96,9 @@ static void setup(void)
> >  static void testfunc_protnone(void)
> >  {
> >         char *addr;
> > -       int page_sz;
> >
> >         sig_caught = 0;
> >
> > -       page_sz = getpagesize();
> > -
> >         addr = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
> >                                          MAP_PRIVATE | MAP_ANONYMOUS, -1,
> >                                          0);
> >
> > @@ -133,7 +130,7 @@ static void testfunc_protnone(void)
> >
> >  #ifdef __ia64__
> >
> > -static char exec_func[] = {
> > +static char exec_func[] __attribute__ ((aligned (64))) = {
> >         0x11, 0x00, 0x00, 0x00, 0x01, 0x00, /* nop.m 0x0             */
> >         0x00, 0x00, 0x00, 0x02, 0x00, 0x80, /* nop.i 0x0             */
> >         0x08, 0x00, 0x84, 0x00,             /* br.ret.sptk.many b0;; */
> > @@ -210,42 +207,33 @@ typedef struct {
> >   * Copy page where &exec_func resides. Also try to copy subsequent page
> >   * in case exec_func is close to page boundary.
> >   */
> > -static void *get_func(void *mem)
> > +static void *get_func(void *mem, uintptr_t *func_page_offset)
> >  {
> >         uintptr_t page_sz = getpagesize();
> >         uintptr_t page_mask = ~(page_sz - 1);
> > -       uintptr_t func_page_offset;
> >         void *func_copy_start, *page_to_copy;
> >         void *mem_start = mem;
> >
> >  #ifdef USE_FUNCTION_DESCRIPTORS
> >         func_descr_t *opd =  (func_descr_t *)&exec_func;
> > -       func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
> > -       func_copy_start = mem + func_page_offset;
> > +       *func_page_offset = (uintptr_t)opd->entry & (page_sz - 1);
> > +       func_copy_start = mem + *func_page_offset;
> >         page_to_copy = (void *)((uintptr_t)opd->entry & page_mask);
> >  #else
> > -       func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
> > -       func_copy_start = mem + func_page_offset;
> > +       *func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
> > +       func_copy_start = mem + *func_page_offset;
> >         page_to_copy = (void *)((uintptr_t)&exec_func & page_mask);
> >  #endif
> > +       tst_resm(TINFO, "exec_func: %p, page_to_copy: %p",
> > +               &exec_func, page_to_copy);
> 
> This was previously inside the body of the if statement right below.
> Not sure if it was intended to always print this information, but I'm
> fine either way.

This was intentional, if we get a failure it seemed like good data.

> 
> >
> >         /* copy 1st page, if it's not present something is wrong */
> > -       if (!page_present(page_to_copy)) {
> > -               tst_resm(TINFO, "exec_func: %p, page_to_copy: %p\n",
> > -                       &exec_func, page_to_copy);
> > +       if (!page_present(page_to_copy))
> >                 tst_brkm(TBROK, cleanup, "page_to_copy not present\n");
> > -       }
> > -       memcpy(mem, page_to_copy, page_sz);
> >
> > -       /* copy 2nd page if possible */
> > -       mem += page_sz;
> > -       page_to_copy += page_sz;
> > -       if (page_present(page_to_copy))
> > -               memcpy(mem, page_to_copy, page_sz);
> > -       else
> > -               memset(mem, 0, page_sz);
> > +       memcpy(mem, page_to_copy, page_sz);
> >
> > -       clear_cache(mem_start, copy_sz);
> > +       clear_cache(mem_start, page_sz);
> >
> >         /* return pointer to area where copy of exec_func resides */
> >         return func_copy_start;
> > @@ -256,23 +244,30 @@ static void *get_func(void *mem)
> >  static void testfunc_protexec(void)
> >  {
> >         func_ptr_t func;
> > +       uintptr_t func_page_offset;
> >         void *p;
> >
> >         sig_caught = 0;
> >
> > -       p = SAFE_MMAP(cleanup, 0, copy_sz, PROT_READ | PROT_WRITE,
> > +       p = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
> >                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> >
> >  #ifdef USE_FUNCTION_DESCRIPTORS
> >         func_descr_t opd;
> > -       opd.entry = (uintptr_t)get_func(p);
> > +       opd.entry = (uintptr_t)get_func(p, &func_page_offset);
> >         func = (func_ptr_t)&opd;
> >  #else
> > -       func = get_func(p);
> > +       func = get_func(p, &func_page_offset);
> >  #endif
> >
> > +       if (func_page_offset + 64 >= page_sz) {
> 
> I'm wondering if this should be ">" not ">=". If the compiler decides
> to use the last 64 bytes in a page and locates the function at offset
> 0xfc0, then that's still ok: 0xfc0 + 0x40 == page_sz

Agreed, I'll make it '>'.

> 
> > +               SAFE_MUNMAP(cleanup, p, page_sz);
> > +               tst_brkm(TCONF, cleanup, "func too close to page boundary,
> > "
> > +                       "maybe your compiler ignores -falign-functions?");
> > +       }
> > +
> >         /* Change the protection to PROT_EXEC. */
> > -       TEST(mprotect(p, copy_sz, PROT_EXEC));
> > +       TEST(mprotect(p, page_sz, PROT_EXEC));
> >
> >         if (TEST_RETURN == -1) {
> >                 tst_resm(TFAIL | TTERRNO, "mprotect failed");
> > @@ -294,7 +289,7 @@ static void testfunc_protexec(void)
> >                 }
> >         }
> >
> > -       SAFE_MUNMAP(cleanup, p, copy_sz);
> > +       SAFE_MUNMAP(cleanup, p, page_sz);
> >  }
> >
> >  static void cleanup(void)
> > --
> > 1.8.3.1
> >
> 

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

* [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes
  2019-02-11 22:15 [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes Jan Stancek
  2019-02-11 23:54 ` Daniel Mentz
@ 2019-02-19 13:29 ` Li Wang
  2019-02-25  7:37   ` Jan Stancek
  1 sibling, 1 reply; 5+ messages in thread
From: Li Wang @ 2019-02-19 13:29 UTC (permalink / raw)
  To: ltp

On Tue, Feb 12, 2019 at 6:15 AM Jan Stancek <jstancek@redhat.com> wrote:

> exec_func() is dummy/empty function. Try to align it so we don't
> need to worry about copying 2 pages. But also check that compiler
> aligned it and there's sufficient space between start of func_exec
> and end of page.
>
> This patch also removes copy_sz, which is now replaced with page_sz.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>

Reviewed-by: Li Wang <liwang@redhat.com>


> +mprotect04: CFLAGS += -falign-functions=64
>

One of my concern was this option maybe not supported by kind of compiler
and probably broken in the compiling phase. So I just submitted these
two(another is Daniel's) patches to Travis CI, the result looks good and no
compiler error found there.
https://travis-ci.com/wangli5665/ltp/builds/101429714

Beside that, I also manually tried that with old gcc(gcc version 4.1.2) and
verify that works fine.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190219/73dc6da9/attachment.html>

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

* [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes
  2019-02-19 13:29 ` Li Wang
@ 2019-02-25  7:37   ` Jan Stancek
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Stancek @ 2019-02-25  7:37 UTC (permalink / raw)
  To: ltp




----- Original Message -----
> On Tue, Feb 12, 2019 at 6:15 AM Jan Stancek <jstancek@redhat.com> wrote:
> 
> > exec_func() is dummy/empty function. Try to align it so we don't
> > need to worry about copying 2 pages. But also check that compiler
> > aligned it and there's sufficient space between start of func_exec
> > and end of page.
> >
> > This patch also removes copy_sz, which is now replaced with page_sz.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> >
> 
> Reviewed-by: Li Wang <liwang@redhat.com>

Pushed.

Regards,
Jan

> 
> 
> > +mprotect04: CFLAGS += -falign-functions=64
> >
> 
> One of my concern was this option maybe not supported by kind of compiler
> and probably broken in the compiling phase. So I just submitted these
> two(another is Daniel's) patches to Travis CI, the result looks good and no
> compiler error found there.
> https://travis-ci.com/wangli5665/ltp/builds/101429714
> 
> Beside that, I also manually tried that with old gcc(gcc version 4.1.2) and
> verify that works fine.
> 
> --
> Regards,
> Li Wang
> 

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

end of thread, other threads:[~2019-02-25  7:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-11 22:15 [LTP] [PATCH v2] syscalls/mprotect: align exec_func to 64 bytes Jan Stancek
2019-02-11 23:54 ` Daniel Mentz
2019-02-12  7:40   ` Jan Stancek
2019-02-19 13:29 ` Li Wang
2019-02-25  7:37   ` Jan Stancek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox