All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func
@ 2015-09-09 11:39 Jan Stancek
  2015-09-09 12:37 ` Cyril Hrubis
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Stancek @ 2015-09-09 11:39 UTC (permalink / raw)
  To: ltp-list

Testcase tried to copy page size area starting at &exec_func.
This results in crash on powerpc, because &exec_func is too close
to end of page and subsequent page is not mapped:

 10000000-10010000 r-xp 00000000 fd:00 402855 mprotect04
 10010000-10020000 rw-p 00000000 fd:00 402855 mprotect04
 806a410000-806a440000 r-xp 00000000 fd:00 2097827 /lib64/ld-2.12.so
where &exec_func == 0x100199c0, and page_size == 65536.

It's also worth noting, that function ptr does not reside in .text,
but in .opd section. That shouldn't matter for this testcase as long
as it doesn't try to copy non-existent pages.

This patch is changing copy function to try copy 2 whole aligned pages.
Both pages are checked to be present in memory before memcpy is
attempted. First is the page which contains &exec_func, and 2nd is the
subsequent page - for the case &exec_func is too close to page boundary.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/mprotect/mprotect04.c | 61 +++++++++++++++++++++----
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/mprotect/mprotect04.c b/testcases/kernel/syscalls/mprotect/mprotect04.c
index 8a4a72ad69f2..23aecbd856f4 100644
--- a/testcases/kernel/syscalls/mprotect/mprotect04.c
+++ b/testcases/kernel/syscalls/mprotect/mprotect04.c
@@ -54,6 +54,7 @@ int TST_TOTAL = ARRAY_SIZE(testfunc);
 
 static volatile int sig_caught;
 static sigjmp_buf env;
+static int copy_sz;
 
 int main(int ac, char **av)
 {
@@ -83,7 +84,9 @@ static void sighandler(int sig)
 
 static void setup(void)
 {
+	tst_tmpdir();
 	tst_sig(NOFORK, sighandler, cleanup);
+	copy_sz = getpagesize() * 2;
 
 	TEST_PAUSE;
 }
@@ -158,32 +161,71 @@ static void exec_func(void)
 	return;
 }
 
-static void *get_func(void *mem)
+static int page_present(void *p)
 {
-	memcpy(mem, exec_func, getpagesize());
+	int fd;
+
+	fd = SAFE_OPEN(cleanup, "page_present", O_WRONLY|O_CREAT, 0644);
+	TEST(write(fd, p, 1));
+	SAFE_CLOSE(cleanup, fd);
+
+	if (TEST_RETURN >= 0)
+		return 1;
+
+	if (TEST_ERRNO != EFAULT)
+		tst_brkm(TBROK | TTERRNO, cleanup, "page_present write");
 
-	return mem;
+	return 0;
+}
+
+/*
+ * 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)
+{
+	uintptr_t page_sz = getpagesize();
+	uintptr_t page_mask = ~(page_sz - 1);
+	uintptr_t func_page_offset = (uintptr_t)&exec_func & (page_sz - 1);
+	void *func_copy_start = mem + func_page_offset;
+	void *page_to_copy = (void *)((uintptr_t)&exec_func & page_mask);
+
+	/* 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);
+		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);
+
+	/* return pointer to area where copy of exec_func resides */
+	return func_copy_start;
 }
 
 #endif
 
 static void testfunc_protexec(void)
 {
-	int page_sz;
 	void (*func)(void);
 	void *p;
 
 	sig_caught = 0;
 
-	page_sz = getpagesize();
-
-	p = SAFE_MMAP(cleanup, 0, page_sz, PROT_READ | PROT_WRITE,
+	p = SAFE_MMAP(cleanup, 0, copy_sz, PROT_READ | PROT_WRITE,
 		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 	func = get_func(p);
 
 	/* Change the protection to PROT_EXEC. */
-	TEST(mprotect(p, page_sz, PROT_EXEC));
+	TEST(mprotect(p, copy_sz, PROT_EXEC));
 
 	if (TEST_RETURN == -1) {
 		tst_resm(TFAIL | TTERRNO, "mprotect failed");
@@ -205,9 +247,10 @@ static void testfunc_protexec(void)
 		}
 	}
 
-	SAFE_MUNMAP(cleanup, p, page_sz);
+	SAFE_MUNMAP(cleanup, p, copy_sz);
 }
 
 static void cleanup(void)
 {
+	tst_rmdir();
 }
-- 
1.8.3.1


------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func
  2015-09-09 11:39 [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func Jan Stancek
@ 2015-09-09 12:37 ` Cyril Hrubis
       [not found]   ` <559206696.7848057.1441803641337.JavaMail.zimbra@redhat.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Cyril Hrubis @ 2015-09-09 12:37 UTC (permalink / raw)
  To: Jan Stancek; +Cc: ltp-list

Hi!
> Testcase tried to copy page size area starting at &exec_func.
> This results in crash on powerpc, because &exec_func is too close
> to end of page and subsequent page is not mapped:

I was wondering about this problem when I was fixing the test, but could
not actually hit it.

> -static void *get_func(void *mem)
> +static int page_present(void *p)
>  {
> -	memcpy(mem, exec_func, getpagesize());
> +	int fd;
> +
> +	fd = SAFE_OPEN(cleanup, "page_present", O_WRONLY|O_CREAT, 0644);

Why don't we use "/dev/null"?


The rest looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func
       [not found]   ` <559206696.7848057.1441803641337.JavaMail.zimbra@redhat.com>
@ 2015-09-09 13:09     ` Cyril Hrubis
       [not found]       ` <870837936.7873040.1441805002410.JavaMail.zimbra@redhat.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Cyril Hrubis @ 2015-09-09 13:09 UTC (permalink / raw)
  To: Jan Stancek; +Cc: ltp-list

Hi!
> It doesn't seem reliable:
> 
> open("/dev/null", O_WRONLY|O_CREAT, 0644) = 3
> write(3, NULL, 1)                       = 1
> close(3)                                = 0
> 
> I'm seeing this on RHEL6.7 kernel, I haven't tried latest upstream yet.

Ah, you are right, the memory is not touched in the write_null()
function at all.

drivers/char/mem.c:

static ssize_t write_null(struct file *file, const char __user *buf,
                          size_t count, loff_t *ppos)
{
        return count;
}

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func
       [not found]       ` <870837936.7873040.1441805002410.JavaMail.zimbra@redhat.com>
@ 2015-09-09 13:26         ` Cyril Hrubis
  0 siblings, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2015-09-09 13:26 UTC (permalink / raw)
  To: Jan Stancek; +Cc: ltp-list

Hi!
> > > I'm seeing this on RHEL6.7 kernel, I haven't tried latest upstream yet.
> > 
> > Ah, you are right, the memory is not touched in the write_null()
> > function at all.
> 
> Right, I got confused by "access_ok(VERIFY_READ, buf, count)"
> in vfs_write(), but after reading code and comments it's clear it
> only checks if range _may_ be valid:
>  * access_ok: - Checks if a user space pointer is valid
>  ...
>  * Returns true (nonzero) if the memory block may be valid, false (zero)
>  * if it is definitely invalid.
>  *
>  * Note that, depending on architecture, this function probably just
>  * checks that the pointer is in the user space range - after calling
>  * this function, memory access functions may still return -EFAULT.

I do not have further questions. Acked.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

end of thread, other threads:[~2015-09-09 13:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-09 11:39 [LTP] [PATCH] mprotect04: fix powerpc crash when copying exec_func Jan Stancek
2015-09-09 12:37 ` Cyril Hrubis
     [not found]   ` <559206696.7848057.1441803641337.JavaMail.zimbra@redhat.com>
2015-09-09 13:09     ` Cyril Hrubis
     [not found]       ` <870837936.7873040.1441805002410.JavaMail.zimbra@redhat.com>
2015-09-09 13:26         ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.