[LTP] [PATCH] Added test for mmap() with MAP_SHARED_VALIDATE.
Paulson Raja L
lpaulsonraja@gmail.com
Sat Mar 25 05:01:39 CET 2023
Hi Petr Vorel,
Thanks for the comments, I have just started contributing to LTP, and
this is my first patch. I agree to merge it as is.
https://github.com/pevik/ltp/commits/paulson/mmap.fixes
https://github.com/pevik/ltp/blob/36ebc4d8900ea8438dc7839f03b7921f0ed9243a/testcases/kernel/syscalls/mmap/mmap20.c
On Fri, Mar 24, 2023 at 4:03 PM Petr Vorel <pvorel@suse.cz> wrote:
> Hi paulson,
>
> Thanks for your patch. Generally the idea LGTM, also the implementation,
> but I suggest various fixes.
>
> There are many fixes (whole diff is below), I put them into my github
> fork, if
> you agree I can merge it as is.
>
> https://github.com/pevik/ltp/commits/paulson/mmap.fixes
>
> https://github.com/pevik/ltp/blob/36ebc4d8900ea8438dc7839f03b7921f0ed9243a/testcases/kernel/syscalls/mmap/mmap20.c
>
> > From: paulson <lpaulsonraja@gmail.com>
> Maybe you want to setup your git better to have your name and surname.
>
> Here is missing in the git commit message (or with fixed name):
> Signed-off-by: paulson <lpaulsonraja@gmail.com>
>
> CI shows failures on CentOS 7:
>
> https://github.com/pevik/ltp/actions/runs/4509751194/jobs/7939880067
>
> /__w/ltp/ltp/testcases/kernel/syscalls/mmap/mmap20.c:48:25: error:
> 'MAP_SHARED_VALIDATE' undeclared (first use in this function)
> INVALID_FLAG | MAP_SHARED_VALIDATE, fd, 0);
> /__w/ltp/ltp/testcases/kernel/syscalls/mmap/mmap20.c:48:25: note: each
> undeclared identifier is reported only once for each function it appears in
>
> There needs to be change in lapi file (which will be used later):
>
> https://patchwork.ozlabs.org/project/ltp/patch/20230324101630.562727-1-pvorel@suse.cz/
>
> You're missing record in runtest/syscalls
>
> mmap20 mmap20
>
> and in .gitignore.
>
> Our checker prints many errors
> cd testcases/kernel/syscalls/mmap; make check-mmap20
>
> mmap20.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in
> line 1
> make: [../../../../include/mk/rules.mk:56: check-mmap20] Error 1 (ignored)
> mmap20.c: note: in included file (through /usr/include/asm-generic/mman.h,
> /usr/include/asm/mman.h, /usr/include/linux/mman.h):
> /usr/include/asm-generic/mman-common.h:26:9: warning: preprocessor token
> MAP_POPULATE redefined
> mmap20.c: note: in included file (through /usr/include/bits/mman.h,
> /usr/include/sys/mman.h):
> /usr/include/bits/mman-map-flags-generic.h:34:10: this was the original
> definition
> ...
>
> will be described later
>
> > ---
> > testcases/kernel/syscalls/mmap/mmap20.c | 61 +++++++++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> > create mode 100644 testcases/kernel/syscalls/mmap/mmap20.c
>
> > diff --git a/testcases/kernel/syscalls/mmap/mmap20.c
> b/testcases/kernel/syscalls/mmap/mmap20.c
> > new file mode 100644
> > index 000000000..ca5bfccd7
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/mmap/mmap20.c
> > @@ -0,0 +1,61 @@
> > +//SPDX-License-Identifier: GPL-2.0-or-later
> Missing space will cause invalid license for tools.
>
> Found by:
> mmap20.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in
> line 1
>
> > +
> > +/*
> > + * Test mmap with MAP_SHARED_VALIDATE flag
> > + *
> > + * We are testing the MAP_SHARED_VALIDATE flag of mmap() syscall. To
> check
> > + * if there is an invalid flag value, the MAP_SHARED_VALIDATE return
> > + * EOPNOTSUPP. The unused bit in the MAP_SHARED_VALIDATE is found, and
> by
> > + * setting the unused bits of the flag argument the flag value becomes
> > + * invalid and the error EOPNOTSUPP is produced as expected.
> > + */
> I'd phrase it as:
>
> /*\
> * [Description]
> *
> * Test mmap(2) with MAP_SHARED_VALIDATE flag.
> *
> * Test expected EOPNOTSUPP errno when testing mmap(2) with
> MAP_SHARED_VALIDATE
> * flag and invalid flag.
> */
>
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
>
> Some
>
> > +#include <sys/mman.h>
> > +#include <linux/mman.h>
>
> 1) mixing <linux/mman.h> and <sys/mman.h> does not work well:
>
> mmap20.c: note: in included file (through /usr/include/asm-generic/mman.h,
> /usr/include/asm/mman.h, /usr/include/linux/mman.h):
> /usr/include/asm-generic/mman-common.h:26:9: warning: preprocessor token
> MAP_POPULATE redefined
> mmap20.c: note: in included file (through /usr/include/bits/mman.h,
> /usr/include/sys/mman.h):
> /usr/include/bits/mman-map-flags-generic.h:34:10: this was the original
> definition
>
>
> I pushed fix
>
> https://github.com/linux-test-project/ltp/commit/32aa5c30c257b2021f9648df186d5b2c7a57dfad
>
> + with my patch
>
> https://patchwork.ozlabs.org/project/ltp/patch/20230324101630.562727-1-pvorel@suse.cz/
>
> you can instead of
>
> > +#include <errno.h>
> > +#include "tst_test.h"
> > +
> > +#define TEST_FILE "file_to_mmap"
> > +#define TEST_FILE_SIZE 1024
> > +#define TEST_FILE_MODE 0600
> File mod is used just once, it's not really important to have it defined.
> But I'd define (1 << 7), because that is expected to be invalid
> (one day this value can appear in <linux/mmap.h> and the test will fail.
>
> > +
> > +static int fd_file = -1;
> > +static void *mapped_address;
> > +
> > +static void setup(void)
> > +{
> > + fd_file = SAFE_OPEN(TEST_FILE, O_CREAT | O_RDWR, TEST_FILE_MODE);
> > + if (tst_fill_file(TEST_FILE, 'a', TEST_FILE_SIZE, 1))
> > + tst_brk(TBROK, "Could not fill the testfile.");
> nit: we don't use dots in the end.
>
> Whole diff is here:
>
> diff --git include/lapi/mmap.h include/lapi/mmap.h
> index 48795369d..7512e9f81 100644
> --- include/lapi/mmap.h
> +++ include/lapi/mmap.h
> @@ -10,6 +10,10 @@
> #include "config.h"
> #include <sys/mman.h>
>
> +#ifndef MAP_SHARED_VALIDATE
> +# define MAP_SHARED_VALIDATE 0x03
> +#endif
> +
> #ifndef MAP_HUGETLB
> # define MAP_HUGETLB 0x40000
> #endif
> diff --git runtest/syscalls runtest/syscalls
> index b9d4a43c8..8b002e989 100644
> --- runtest/syscalls
> +++ runtest/syscalls
> @@ -794,6 +794,7 @@ mmap16 mmap16
> mmap17 mmap17
> mmap18 mmap18
> mmap19 mmap19
> +mmap20 mmap20
>
> modify_ldt01 modify_ldt01
> modify_ldt02 modify_ldt02
> diff --git testcases/kernel/syscalls/mmap/.gitignore
> testcases/kernel/syscalls/mmap/.gitignore
> index 8811226be..569a76ac1 100644
> --- testcases/kernel/syscalls/mmap/.gitignore
> +++ testcases/kernel/syscalls/mmap/.gitignore
> @@ -18,3 +18,4 @@
> /mmap17
> /mmap18
> /mmap19
> +/mmap20
> diff --git testcases/kernel/syscalls/mmap/mmap20.c
> testcases/kernel/syscalls/mmap/mmap20.c
> index ca5bfccd7..54ddb2087 100644
> --- testcases/kernel/syscalls/mmap/mmap20.c
> +++ testcases/kernel/syscalls/mmap/mmap20.c
> @@ -1,55 +1,58 @@
> -//SPDX-License-Identifier: GPL-2.0-or-later
> -
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - * Test mmap with MAP_SHARED_VALIDATE flag
> + * Copyright (c) 2023 paulson <lpaulsonraja@gmail.com>
> + */
> +
> +/*\
> + * [Description]
> *
> - * We are testing the MAP_SHARED_VALIDATE flag of mmap() syscall. To check
> - * if there is an invalid flag value, the MAP_SHARED_VALIDATE return
> - * EOPNOTSUPP. The unused bit in the MAP_SHARED_VALIDATE is found, and by
> - * setting the unused bits of the flag argument the flag value becomes
> - * invalid and the error EOPNOTSUPP is produced as expected.
> + * Test mmap(2) with MAP_SHARED_VALIDATE flag.
> + *
> + * Test expected EOPNOTSUPP errno when testing mmap(2) with
> MAP_SHARED_VALIDATE
> + * flag and invalid flag.
> */
> +
> +#include <errno.h>
> #include <stdio.h>
> #include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
> -#include <sys/mman.h>
> -#include <linux/mman.h>
> -#include <errno.h>
> #include "tst_test.h"
> +#include "lapi/mmap.h"
>
> #define TEST_FILE "file_to_mmap"
> #define TEST_FILE_SIZE 1024
> -#define TEST_FILE_MODE 0600
> +#define INVALID_FLAG (1 << 7)
>
> -static int fd_file = -1;
> -static void *mapped_address;
> +static int fd = -1;
> +static void *addr;
>
> static void setup(void)
> {
> - fd_file = SAFE_OPEN(TEST_FILE, O_CREAT | O_RDWR, TEST_FILE_MODE);
> + fd = SAFE_OPEN(TEST_FILE, O_CREAT | O_RDWR, 0600);
> +
> if (tst_fill_file(TEST_FILE, 'a', TEST_FILE_SIZE, 1))
> - tst_brk(TBROK, "Could not fill the testfile.");
> + tst_brk(TBROK, "Could not fill the testfile");
> }
>
> static void cleanup(void)
> {
> - if (fd_file > -1)
> - SAFE_CLOSE(fd_file);
> - if (mapped_address != NULL && mapped_address != MAP_FAILED)
> - SAFE_MUNMAP(mapped_address, TEST_FILE_SIZE);
> + if (fd > -1)
> + SAFE_CLOSE(fd);
> +
> + if (addr && addr != MAP_FAILED)
> + SAFE_MUNMAP(addr, TEST_FILE_SIZE);
> }
>
> static void test_mmap(void)
> {
> - mapped_address = mmap(NULL, TEST_FILE_SIZE, PROT_READ | PROT_WRITE,
> - (1 << 7) | MAP_SHARED_VALIDATE, fd_file, 0);
> - if (mapped_address != MAP_FAILED)
> - tst_res(TFAIL | TERRNO, "mmap() is successful, but it
> should have failed.");
> + addr = mmap(NULL, TEST_FILE_SIZE, PROT_READ | PROT_WRITE,
> + INVALID_FLAG | MAP_SHARED_VALIDATE, fd, 0);
> +
> + if (addr != MAP_FAILED)
> + tst_res(TFAIL | TERRNO, "mmap() is successful, but it
> should have failed");
> else if (errno == EOPNOTSUPP)
> - tst_res(TPASS, "mmap() failed with errno set to
> EOPNOTSUPP.");
> + tst_res(TPASS, "mmap() failed with errno set to
> EOPNOTSUPP");
> else
> - tst_res(TFAIL | TERRNO, "mmap() failed with unexpected
> error.");
> + tst_res(TFAIL | TERRNO, "mmap() failed with unexpected
> error");
> }
>
> static struct tst_test test = {
>
More information about the ltp
mailing list