Re: [LTP] RFC:[PATCH v1] Added testcase to test mmap with MAP_SHARED_VALIDATE flag

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

In principal the test is good, you can remove the RFC.

Please see comments below (in addition to Cyril's).

Paulson Raja L <lpaulsonraja@gmail.com> writes:

> This patch adds a new test case for mmap syscall. It tests the
> MAP_SHARED_VALIDATE flag of mmap. The code checks of the
> MAP_SHARED_VALIDATE flag return EOPNOTSUPP when mapped with an invalid flag
> value. It does so by setting the unused bits of the flag argument.

Would it be possible to use two incompatible flags together instead?

Unused flags can become valid at a later date.

>
> Signed-off-by: Paulson Raja L. <lpaulsonraja@gmail.com>
>
> diff --git a/testcases/kernel/syscalls/mmap/mmap20.c
> b/testcases/kernel/syscalls/mmap/mmap20.c
> new file mode 100644
> index 000000000..2f6dd5d4d
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mmap/mmap20.c
> @@ -0,0 +1,61 @@
> +//SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * 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.
> + */
> +#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"
> +
> +#define TEST_FILE "file_to_mmap"
> +#define TEST_FILE_SIZE 1024
> +#define TEST_FILE_MODE 0600
> +
> +static int fd_file;
> +static void *mapped_address = NULL;
> +
> +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.");
> +}
> +
> +static void cleanup(void)
> +{
> + if (fd_file > 0)

It's unlikely, but possible that fd_file is 0, so it should be
initialised to -1 and we check it is > -1.

> + SAFE_CLOSE(fd_file);
> + if (mapped_address != NULL && mapped_address != MAP_FAILED)
> + SAFE_MUNMAP(mapped_address, TEST_FILE_SIZE);
> +}
> +
> +static void test_mmap(void)
> +{
> + mapped_address = mmap(NULL, TEST_FILE_SIZE, PROT_READ | PROT_WRITE,
> +      (1 << 7) | (1 << 9) | MAP_SHARED_VALIDATE, fd_file, 0);

The TEST macro can be used here, which will set errno=0. Possibly errno
is already set to some previous error. (also note you need to then use
TTERRNO below)

> + if (mapped_address != 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.");
> + else
> + tst_res(TFAIL | TERRNO, "mmap() failed with unexpected error.");
> +}
> +
> +static struct tst_test test = {
> + .min_kver = "4.15",
> + .setup = setup,
> + .cleanup = cleanup,
> + .test_all = test_mmap,
> + .needs_tmpdir = 1,
> +};



-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp