[LTP] [PATCH] Added test for mmap() with MAP_SHARED_VALIDATE.
Petr Vorel
pvorel@suse.cz
Fri Mar 24 11:33:34 CET 2023
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