From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cyril Hrubis Date: Tue, 30 Jun 2020 11:55:54 +0200 Subject: [LTP] [PATCH v1] Add a test case for mmap() MAP_GROWSDOWN flag In-Reply-To: <20200630054405.6115-1-pravinraghul@zilogic.com> References: <20200630054405.6115-1-pravinraghul@zilogic.com> Message-ID: <20200630095554.GB2684@yuki.lan> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it Hi! > We assign the memory region allocated using MAP_GROWSDOWN to a thread, > as a stack, to test the effect of MAP_GROWSDOWN. This is because the > kernel only grows the memory region when the stack pointer, is within > guard page, when the guard page is touched. > > 1. Map an anyonymous memory region of size X, and unmap it. > 2. Split the unmapped memory region into two. > 3. The lower memory region is left unmapped. > 4. The higher memory region is mapped for use as stack, using MAP_FIXED | MAP_GROWSDOWN. > 5. The higher memory region is provided as stack to a thread, where > a recursive function is invoked. > 6. The stack grows beyond the allocated region, into the lower memory area. > 7. If this results in the memory region being extended, into the > unmapped region, the test is considered to have passed. > > Resolves #300 > Signed-off-by: Pravin Raghul S. > Reviewed-by: Vijay Kumar B. > > --- > runtest/syscalls | 1 + > testcases/kernel/syscalls/mmap/.gitignore | 1 + > testcases/kernel/syscalls/mmap/Makefile | 7 ++ > testcases/kernel/syscalls/mmap/mmap18.c | 146 ++++++++++++++++++++++ > 4 files changed, 155 insertions(+) > create mode 100644 testcases/kernel/syscalls/mmap/mmap18.c > > diff --git a/runtest/syscalls b/runtest/syscalls > index b4d523319..d8c9dbe92 100644 > --- a/runtest/syscalls > +++ b/runtest/syscalls > @@ -747,6 +747,7 @@ mmap14 mmap14 > mmap15 mmap15 > mmap16 mmap16 > mmap17 mmap17 > +mmap18 mmap18 > > modify_ldt01 modify_ldt01 > modify_ldt02 modify_ldt02 > diff --git a/testcases/kernel/syscalls/mmap/.gitignore b/testcases/kernel/syscalls/mmap/.gitignore > index c5c083d4b..4fd90ab5f 100644 > --- a/testcases/kernel/syscalls/mmap/.gitignore > +++ b/testcases/kernel/syscalls/mmap/.gitignore > @@ -16,3 +16,4 @@ > /mmap15 > /mmap16 > /mmap17 > +/mmap18 > diff --git a/testcases/kernel/syscalls/mmap/Makefile b/testcases/kernel/syscalls/mmap/Makefile > index 743ca36e7..bdc49e4be 100644 > --- a/testcases/kernel/syscalls/mmap/Makefile > +++ b/testcases/kernel/syscalls/mmap/Makefile > @@ -8,3 +8,10 @@ include $(top_srcdir)/include/mk/testcases.mk > include $(top_srcdir)/include/mk/generic_leaf_target.mk > > LDLIBS += -lpthread > +# > +# We use recursive calls to to grow the stack, to test the > +# MAP_GROSWDOWN flag. But tail call optimization by the compiler > +# can prevent the recusive call and stack growth. Disable tail > +# call optmization using -fno-optimize-sibling-calls > +# > +mmap18: CFLAGS += -fno-optimize-sibling-calls > diff --git a/testcases/kernel/syscalls/mmap/mmap18.c b/testcases/kernel/syscalls/mmap/mmap18.c > new file mode 100644 > index 000000000..01a456bfc > --- /dev/null > +++ b/testcases/kernel/syscalls/mmap/mmap18.c > @@ -0,0 +1,146 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) Zilogic Systems Pvt. Ltd., 2020 > + * Email: code@zilogic.com > + */ > + > +/* > + * Test mmap() MAP_GROWSDOWN flag > + * > + * We assign the memory region allocated using MAP_GROWSDOWN to a thread, > + * as a stack, to test the effect of MAP_GROWSDOWN. This is because the > + * kernel only grows the memory region when the stack pointer, is within > + * guard page, when the guard page is touched. > + * > + * 1. Map an anyonymous memory region of size X, and unmap it. > + * 2. Split the unmapped memory region into two. > + * 3. The lower memory region is left unmapped. > + * 4. The higher memory region is mapped for use as stack, using ^ Trailing whitespace. You can use the checkpatch.pl from Linux kernel git repostory to check for minor mistakes like these before sending a patch. > + * MAP_FIXED | MAP_GROWSDOWN. > + * 5. The higher memory region is provided as stack to a thread, where > + * a recursive function is invoked. > + * 6. The stack grows beyond the allocated region, into the lower memory area. > + * 7. If this results in the memory region being extended, into the > + * unmapped region, the test is considered to have passed. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "tst_test.h" > +#include "tst_safe_pthread.h" > + > +#define UNITS(x) ((x) * PTHREAD_STACK_MIN) > + > +static void *stack = NULL; Global variables are zeroed at the program start, there is no point to set them to NULL like that. > +bool check_stackgrow_up(int *local_var_1) Missing static. > +{ > + int local_var_2; > + > + if (local_var_1 < &local_var_2) > + return false; > + else > + return true; Just do return !(local_var_1 < &local_var_2); > +} > + > +void setup(void) Here as well. > +{ > + int local_var_1; > + bool stackgrow_up; > + > + stackgrow_up = check_stackgrow_up(&local_var_1); > + if (stackgrow_up) Why not just if (stackgrow_up(&local_var_1)) here instead? > + tst_brk(TCONF, "Test can't be performed with stack grows up architecture"); > +} > + > +void cleanup(void) And here as well. > +{ > + if (stack != NULL) Just if (stack) > + SAFE_MUNMAP(stack, UNITS(8)); > +} > + > +void *find_free_range(size_t size) Here as well. > +{ > + void *mem; > + > + mem = SAFE_MMAP(NULL, size, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + SAFE_MUNMAP(mem, size); > + > + return mem; > +} > + > +void split_unmapped_plus_stack(void *start, size_t size, void **stack) Here as well. > +{ > + /* > + * +---------------------+----------------------+ > + * + unmapped | stack | > + * +---------------------+----------------------+ > + */ > + *stack = SAFE_MMAP(start, size, PROT_READ | PROT_WRITE, > + MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN, > + -1, 0); Why can't this function return void * instead? Also I do not get what the ascii art means here. We just mmap(0 the whole thing here with MAP_GROWSDOWN what do we split? Also why can't we just use multiple of page_size instead of some pthread magic constant? Why don't we just allocate 10 pages then touch them in order? > +} > + > +static void *check_depth_recursive(void *limit) > +{ > + if ((off_t) &limit < (off_t) limit) > + return NULL; > + > + return check_depth_recursive(limit); > +} > + > +void grow_stack(void *stack, size_t size, void *limit) > +{ > + pthread_t test_thread; > + pthread_attr_t attr; > + int ret; > + > + ret = pthread_attr_init(&attr); > + if (ret != 0) > + tst_brk(TBROK, "pthread_attr_init failed during setup"); > + > + ret = pthread_attr_setstack(&attr, stack, size); > + if (ret != 0) > + tst_brk(TBROK, "pthread_attr_setstack failed during setup"); > + > + SAFE_PTHREAD_CREATE(&test_thread, &attr, check_depth_recursive, limit); > + SAFE_PTHREAD_JOIN(test_thread, NULL); Do we really have to use pthreads here? Can't we just touch the guard page by writing to it? I guess that we can just do for () loop over the mapping with something as: for (i = 0; i < size; i++) ((char*)stack)[i] = 'a'; > + exit(0); > +} > + > +static void run_test(void) > +{ > + void *mem; > + pid_t child_pid; > + int wstatus; > + > + mem = find_free_range(UNITS(16)); > + split_unmapped_plus_stack(mem, UNITS(16), &stack); > + > + child_pid = SAFE_FORK(); > + if (child_pid == 0) { Use just if (!child_pid) > + grow_stack(stack, UNITS(8), mem + UNITS(1)); > + } else { The grow_stack() function calls exit(0) there is no point in having the else branch here. > + SAFE_WAIT(&wstatus); > + if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0) > + tst_res(TPASS, "stack grows in unmapped region"); > + else > + tst_res(TFAIL, "child exited with %d", wstatus); > + } No test result propagation, the child should report the test result using the tst_res() itself. > +} > + > +static struct tst_test test = { > + .setup = setup, > + .cleanup = cleanup, > + .test_all = run_test, > + .forks_child = 1, > +}; -- Cyril Hrubis chrubis@suse.cz