Message ID | 20240124132603.16199-1-akumar@suse.de |
---|---|
State | Accepted |
Headers | show |
Series | [v2] mmap04.c: Avoid vma merging | expand |
Hi, Reviewed-by: Martin Doucha <mdoucha@suse.cz> On 24. 01. 24 14:26, Avinesh Kumar wrote: > We hit a scenario where new mapping was merged with existing mapping of > same permission and the return address from the mmap was hidden in the > merged mapping in /proc/self/maps, causing the test to fail. > To avoid this, we first create a 2-page mapping with the different > permissions, and then remap the 2nd page with the perms being tested. > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > Reported-by: Martin Doucha <mdoucha@suse.cz> > Signed-off-by: Avinesh Kumar <akumar@suse.de> > --- > testcases/kernel/syscalls/mmap/mmap04.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/testcases/kernel/syscalls/mmap/mmap04.c b/testcases/kernel/syscalls/mmap/mmap04.c > index f6f4f7c98..fa85deed1 100644 > --- a/testcases/kernel/syscalls/mmap/mmap04.c > +++ b/testcases/kernel/syscalls/mmap/mmap04.c > @@ -17,8 +17,8 @@ > #include "tst_test.h" > #include <stdio.h> > > -#define MMAPSIZE 1024 > -static char *addr; > +static char *addr1; > +static char *addr2; > > static struct tcase { > int prot; > @@ -44,14 +44,23 @@ static struct tcase { > static void run(unsigned int i) > { > struct tcase *tc = &tcases[i]; > - char addr_str[20]; > char perms[8]; > char fmt[1024]; > + unsigned int pagesize; > + int flag; > > - addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0); > + pagesize = SAFE_SYSCONF(_SC_PAGESIZE); > > - sprintf(addr_str, "%" PRIxPTR, (uintptr_t)addr); > - sprintf(fmt, "%s-%%*x %%s", addr_str); > + /* To avoid new mapping getting merged with existing mappings, we first > + * create a 2-page mapping with the different permissions, and then remap > + * the 2nd page with the perms being tested. > + */ > + flag = (tc->flags & MAP_PRIVATE) ? MAP_SHARED : MAP_PRIVATE; > + addr1 = SAFE_MMAP(NULL, pagesize * 2, PROT_NONE, MAP_ANONYMOUS | flag, -1, 0); > + > + addr2 = SAFE_MMAP(addr1 + pagesize, pagesize, tc->prot, tc->flags | MAP_FIXED, -1, 0); > + > + sprintf(fmt, "%" PRIxPTR "-%%*x %%s", (uintptr_t)addr2); > SAFE_FILE_LINES_SCANF("/proc/self/maps", fmt, perms); > > if (!strcmp(perms, tc->exp_perms)) { > @@ -61,7 +70,7 @@ static void run(unsigned int i) > tc->exp_perms, perms); > } > > - SAFE_MUNMAP(addr, MMAPSIZE); > + SAFE_MUNMAP(addr1, pagesize * 2); > } > > static struct tst_test test = {
Hi Avinesh, Martin, > Hi, > Reviewed-by: Martin Doucha <mdoucha@suse.cz> > On 24. 01. 24 14:26, Avinesh Kumar wrote: > > We hit a scenario where new mapping was merged with existing mapping of > > same permission and the return address from the mmap was hidden in the > > merged mapping in /proc/self/maps, causing the test to fail. > > To avoid this, we first create a 2-page mapping with the different > > permissions, and then remap the 2nd page with the perms being tested. > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > Reported-by: Martin Doucha <mdoucha@suse.cz> > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > --- > > testcases/kernel/syscalls/mmap/mmap04.c | 23 ++++++++++++++++------- > > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/testcases/kernel/syscalls/mmap/mmap04.c b/testcases/kernel/syscalls/mmap/mmap04.c > > index f6f4f7c98..fa85deed1 100644 > > --- a/testcases/kernel/syscalls/mmap/mmap04.c > > +++ b/testcases/kernel/syscalls/mmap/mmap04.c > > @@ -17,8 +17,8 @@ > > #include "tst_test.h" > > #include <stdio.h> > > -#define MMAPSIZE 1024 > > -static char *addr; > > +static char *addr1; > > +static char *addr2; > > static struct tcase { > > int prot; > > @@ -44,14 +44,23 @@ static struct tcase { > > static void run(unsigned int i) > > { > > struct tcase *tc = &tcases[i]; > > - char addr_str[20]; > > char perms[8]; > > char fmt[1024]; > > + unsigned int pagesize; > > + int flag; > > - addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0); > > + pagesize = SAFE_SYSCONF(_SC_PAGESIZE); > > - sprintf(addr_str, "%" PRIxPTR, (uintptr_t)addr); > > - sprintf(fmt, "%s-%%*x %%s", addr_str); > > + /* To avoid new mapping getting merged with existing mappings, we first > > + * create a 2-page mapping with the different permissions, and then remap > > + * the 2nd page with the perms being tested. > > + */ > > + flag = (tc->flags & MAP_PRIVATE) ? MAP_SHARED : MAP_PRIVATE; > > + addr1 = SAFE_MMAP(NULL, pagesize * 2, PROT_NONE, MAP_ANONYMOUS | flag, -1, 0); > > + > > + addr2 = SAFE_MMAP(addr1 + pagesize, pagesize, tc->prot, tc->flags | MAP_FIXED, -1, 0); > > + > > + sprintf(fmt, "%" PRIxPTR "-%%*x %%s", (uintptr_t)addr2); > > SAFE_FILE_LINES_SCANF("/proc/self/maps", fmt, perms); > > if (!strcmp(perms, tc->exp_perms)) { > > @@ -61,7 +70,7 @@ static void run(unsigned int i) > > tc->exp_perms, perms); > > } > > - SAFE_MUNMAP(addr, MMAPSIZE); > > + SAFE_MUNMAP(addr1, pagesize * 2); Shouldn't there be also second munmap()? SAFE_MUNMAP(addr2, pagesize); Kind regards, Petr > > } > > static struct tst_test test = {
Hi Petr, On Wednesday, January 24, 2024 6:05:47 PM CET Petr Vorel wrote: > Hi Avinesh, Martin, > > > Hi, > > Reviewed-by: Martin Doucha <mdoucha@suse.cz> > > > > On 24. 01. 24 14:26, Avinesh Kumar wrote: > > > We hit a scenario where new mapping was merged with existing mapping of > > > same permission and the return address from the mmap was hidden in the > > > merged mapping in /proc/self/maps, causing the test to fail. > > > To avoid this, we first create a 2-page mapping with the different > > > permissions, and then remap the 2nd page with the perms being tested. > > > > > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > > Reported-by: Martin Doucha <mdoucha@suse.cz> > > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > > --- > > > > > > testcases/kernel/syscalls/mmap/mmap04.c | 23 ++++++++++++++++------- > > > 1 file changed, 16 insertions(+), 7 deletions(-) > > > > > > diff --git a/testcases/kernel/syscalls/mmap/mmap04.c > > > b/testcases/kernel/syscalls/mmap/mmap04.c index f6f4f7c98..fa85deed1 > > > 100644 > > > --- a/testcases/kernel/syscalls/mmap/mmap04.c > > > +++ b/testcases/kernel/syscalls/mmap/mmap04.c > > > @@ -17,8 +17,8 @@ > > > > > > #include "tst_test.h" > > > #include <stdio.h> > > > > > > -#define MMAPSIZE 1024 > > > -static char *addr; > > > +static char *addr1; > > > +static char *addr2; > > > > > > static struct tcase { > > > > > > int prot; > > > > > > @@ -44,14 +44,23 @@ static struct tcase { > > > > > > static void run(unsigned int i) > > > { > > > > > > struct tcase *tc = &tcases[i]; > > > > > > - char addr_str[20]; > > > > > > char perms[8]; > > > char fmt[1024]; > > > > > > + unsigned int pagesize; > > > + int flag; > > > - addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0); > > > + pagesize = SAFE_SYSCONF(_SC_PAGESIZE); > > > - sprintf(addr_str, "%" PRIxPTR, (uintptr_t)addr); > > > - sprintf(fmt, "%s-%%*x %%s", addr_str); > > > + /* To avoid new mapping getting merged with existing mappings, we > > > first > > > + * create a 2-page mapping with the different permissions, and then > > > remap > > > + * the 2nd page with the perms being tested. > > > + */ > > > + flag = (tc->flags & MAP_PRIVATE) ? MAP_SHARED : MAP_PRIVATE; > > > + addr1 = SAFE_MMAP(NULL, pagesize * 2, PROT_NONE, MAP_ANONYMOUS | flag, > > > -1, 0); + > > > + addr2 = SAFE_MMAP(addr1 + pagesize, pagesize, tc->prot, tc->flags | > > > MAP_FIXED, -1, 0); + > > > + sprintf(fmt, "%" PRIxPTR "-%%*x %%s", (uintptr_t)addr2); > > > > > > SAFE_FILE_LINES_SCANF("/proc/self/maps", fmt, perms); > > > if (!strcmp(perms, tc->exp_perms)) { > > > > > > @@ -61,7 +70,7 @@ static void run(unsigned int i) > > > > > > tc- >exp_perms, perms); > > > > > > } > > > > > > - SAFE_MUNMAP(addr, MMAPSIZE); > > > + SAFE_MUNMAP(addr1, pagesize * 2); > > Shouldn't there be also second munmap()? > SAFE_MUNMAP(addr2, pagesize); No, we are unmapping both the mappings ( 2 pages ) together. Regards, Avinesh > > Kind regards, > Petr > > > > } > > > static struct tst_test test = {
Hi Avinesh, Martin, > > Shouldn't there be also second munmap()? > > SAFE_MUNMAP(addr2, pagesize); > No, we are unmapping both the mappings ( 2 pages ) together. Ah, thanks! Merged. Kind regards, Petr
diff --git a/testcases/kernel/syscalls/mmap/mmap04.c b/testcases/kernel/syscalls/mmap/mmap04.c index f6f4f7c98..fa85deed1 100644 --- a/testcases/kernel/syscalls/mmap/mmap04.c +++ b/testcases/kernel/syscalls/mmap/mmap04.c @@ -17,8 +17,8 @@ #include "tst_test.h" #include <stdio.h> -#define MMAPSIZE 1024 -static char *addr; +static char *addr1; +static char *addr2; static struct tcase { int prot; @@ -44,14 +44,23 @@ static struct tcase { static void run(unsigned int i) { struct tcase *tc = &tcases[i]; - char addr_str[20]; char perms[8]; char fmt[1024]; + unsigned int pagesize; + int flag; - addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0); + pagesize = SAFE_SYSCONF(_SC_PAGESIZE); - sprintf(addr_str, "%" PRIxPTR, (uintptr_t)addr); - sprintf(fmt, "%s-%%*x %%s", addr_str); + /* To avoid new mapping getting merged with existing mappings, we first + * create a 2-page mapping with the different permissions, and then remap + * the 2nd page with the perms being tested. + */ + flag = (tc->flags & MAP_PRIVATE) ? MAP_SHARED : MAP_PRIVATE; + addr1 = SAFE_MMAP(NULL, pagesize * 2, PROT_NONE, MAP_ANONYMOUS | flag, -1, 0); + + addr2 = SAFE_MMAP(addr1 + pagesize, pagesize, tc->prot, tc->flags | MAP_FIXED, -1, 0); + + sprintf(fmt, "%" PRIxPTR "-%%*x %%s", (uintptr_t)addr2); SAFE_FILE_LINES_SCANF("/proc/self/maps", fmt, perms); if (!strcmp(perms, tc->exp_perms)) { @@ -61,7 +70,7 @@ static void run(unsigned int i) tc->exp_perms, perms); } - SAFE_MUNMAP(addr, MMAPSIZE); + SAFE_MUNMAP(addr1, pagesize * 2); } static struct tst_test test = {