[LTP] [PATCH 1/4] lib: Add SAFE_CALLOC() helper function to LTP library

[LTP] [PATCH 1/4] lib: Add SAFE_CALLOC() helper function to LTP library

[Avinesh Kumar]

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 include/tst_safe_macros.h |  5 +++++
 lib/tst_safe_macros.c     | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 0cf3d7878..90ef451fd 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -72,6 +72,11 @@ int safe_dup2(const char *file, const int lineno, int oldfd, int newfd);
 #define SAFE_MALLOC(size) \
 	safe_malloc(__FILE__, __LINE__, NULL, (size))
 
+void *safe_calloc(const char *file, const int lineno, size_t nmemb, size_t size);
+
+#define SAFE_CALLOC(nmemb, size) \
+	safe_calloc(__FILE__, __LINE__, (nmemb), (size))
+
 void *safe_realloc(const char *file, const int lineno, void *ptr, size_t size);
 
 #define SAFE_REALLOC(ptr, size) \
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index c4cdc87e7..f69802726 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -548,6 +548,21 @@ int safe_dup2(const char *file, const int lineno, int oldfd, int newfd)
 	return rval;
 }
 
+void *safe_calloc(const char *file, const int lineno,
+				size_t nmemb, size_t size)
+{
+	void *rval;
+
+	rval = calloc(nmemb, size);
+
+	if (rval == NULL) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"calloc(%zu, %zu) failed", nmemb, size);
+	}
+
+	return rval;
+}
+
 void *safe_realloc(const char *file, const int lineno, void *ptr, size_t size)
 {
 	void *ret;
-- 
2.41.0


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

[LTP] [PATCH 2/4] syscalls/mmap01: Rewrite the test using new LTP API

[Avinesh Kumar]

also old test was broken for iterations > 1 as mmap() returns the same
mapping address each time and we need to clear the memory contents in
every loop for test to work correctly.

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap01.c | 219 +++++++-----------------
 1 file changed, 58 insertions(+), 161 deletions(-)

diff --git a/testcases/kernel/syscalls/mmap/mmap01.c b/testcases/kernel/syscalls/mmap/mmap01.c
index 99266b57f..84950a2ac 100644
--- a/testcases/kernel/syscalls/mmap/mmap01.c
+++ b/testcases/kernel/syscalls/mmap/mmap01.c
@@ -1,194 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
- *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY;  without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *	07/2001 Ported by Wayne Boyer
+ * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
  */
 
-/*
- * Test Description:
- *  Verify that, mmap() succeeds when used to map a file where size of the
- *  file is not a multiple of the page size, the memory area beyond the end
- *  of the file to the end of the page is accessible. Also, verify that
- *  this area is all zeroed and the modifications done to this area are
- *  not written to the file.
+/*\
+ * [Description]
  *
- * Expected Result:
- *  mmap() should succeed returning the address of the mapped region.
- *  The memory area beyond the end of file to the end of page should be
- *  filled with zero.
- *  The changes beyond the end of file should not get written to the file.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
+ * Verify that, mmap() succeeds when used to map a file where size of the
+ * file is not a multiple of the page size, the memory area beyond the end
+ * of the file to the end of the page is accessible. Also, verify that
+ * this area is all zeroed and the modifications done to this area are
+ * not written to the file.
  */
+
 #include <stdio.h>
 #include <stdlib.h>
-#include <sys/types.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <string.h>
-#include <signal.h>
-#include <stdint.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-#include <sys/shm.h>
-
-#include "test.h"
+#include "tst_test.h"
 
-#define TEMPFILE	"mmapfile"
-
-char *TCID = "mmap01";
-int TST_TOTAL = 1;
-
-static char *addr;
-static char *dummy;
+#define TEMPFILE "mmapfile"
+static int fd;
 static size_t page_sz;
 static size_t file_sz;
-static int fildes;
+static char *dummy;
+static char *addr;
 static char cmd_buffer[BUFSIZ];
 
-static void setup(void);
-static void cleanup(void);
-
-int main(int ac, char **av)
-{
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/*
-		 * Call mmap to map the temporary file beyond EOF
-		 * with write access.
-		 */
-		errno = 0;
-		addr = mmap(NULL, page_sz, PROT_READ | PROT_WRITE,
-			    MAP_FILE | MAP_SHARED, fildes, 0);
-
-		/* Check for the return value of mmap() */
-		if (addr == MAP_FAILED) {
-			tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE);
-			continue;
-		}
-
-		/*
-		 * Check if mapped memory area beyond EOF are
-		 * zeros and changes beyond EOF are not written
-		 * to file.
-		 */
-		if (memcmp(&addr[file_sz], dummy, page_sz - file_sz)) {
-			tst_brkm(TFAIL, cleanup,
-				 "mapped memory area contains invalid "
-				 "data");
-		}
-
-		/*
-		 * Initialize memory beyond file size
-		 */
-		addr[file_sz] = 'X';
-		addr[file_sz + 1] = 'Y';
-		addr[file_sz + 2] = 'Z';
-
-		/*
-		 * Synchronize the mapped memory region
-		 * with the file.
-		 */
-		if (msync(addr, page_sz, MS_SYNC) != 0) {
-			tst_brkm(TFAIL | TERRNO, cleanup,
-				 "failed to synchronize mapped file");
-		}
-
-		/*
-		 * Now, Search for the pattern 'XYZ' in the
-		 * temporary file.  The pattern should not be
-		 * found and the return value should be 1.
-		 */
-		if (system(cmd_buffer) != 0) {
-			tst_resm(TPASS,
-				 "Functionality of mmap() successful");
-		} else {
-			tst_resm(TFAIL,
-				 "Specified pattern found in file");
-		}
-
-		/* Clean up things in case we are looping */
-		/* Unmap the mapped memory */
-		if (munmap(addr, page_sz) != 0) {
-			tst_brkm(TFAIL | TERRNO, NULL, "munmap failed");
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
 static void setup(void)
 {
 	struct stat stat_buf;
-	char Path_name[PATH_MAX];
+	char path_name[PATH_MAX];
 	char write_buf[] = "hello world\n";
 
-	tst_sig(FORK, DEF_HANDLER, cleanup);
+	SAFE_GETCWD(path_name, sizeof(path_name));
 
-	TEST_PAUSE;
+	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
 
-	tst_tmpdir();
+	SAFE_WRITE(SAFE_WRITE_ALL, fd, write_buf, strlen(write_buf));
+	SAFE_STAT(TEMPFILE, &stat_buf);
 
-	/* Get the path of temporary file to be created */
-	if (getcwd(Path_name, sizeof(Path_name)) == NULL) {
-		tst_brkm(TFAIL | TERRNO, cleanup,
-			 "getcwd failed to get current working directory");
-	}
+	file_sz = stat_buf.st_size;
+	page_sz = getpagesize();
 
-	/* Creat a temporary file used for mapping */
-	if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) {
-		tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE);
-	}
+	dummy = SAFE_CALLOC(page_sz, sizeof(char));
 
-	/* Write some data into temporary file */
-	if (write(fildes, write_buf, strlen(write_buf)) != (long)strlen(write_buf)) {
-		tst_brkm(TFAIL, cleanup, "writing to %s", TEMPFILE);
-	}
+	sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", path_name, TEMPFILE);
+}
 
-	/* Get the size of temporary file */
-	if (stat(TEMPFILE, &stat_buf) < 0) {
-		tst_brkm(TFAIL | TERRNO, cleanup, "stat of %s failed",
-			 TEMPFILE);
+static void run(void)
+{
+	addr = mmap(NULL, page_sz, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0);
+	if (addr == MAP_FAILED) {
+		tst_res(TFAIL | TERRNO, "mmap() of %s failed", TEMPFILE);
+		return;
 	}
-	file_sz = stat_buf.st_size;
 
-	page_sz = getpagesize();
+	if (memcmp(&addr[file_sz], dummy, page_sz - file_sz) != 0)
+		tst_brk(TFAIL, "mapped memory area contains invalid data");
 
-	/* Allocate and initialize dummy string of system page size bytes */
-	if ((dummy = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, cleanup, "calloc failed (dummy)");
-	}
+	addr[file_sz] = 'X';
+	addr[file_sz + 1] = 'Y';
+	addr[file_sz + 2] = 'Z';
+
+	if (msync(addr, page_sz, MS_SYNC) != 0)
+		tst_brk(TFAIL | TERRNO, "failed to sync mapped file");
 
-	/* Create the command which will be executed in the test */
-	sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", Path_name, TEMPFILE);
+	if (system(cmd_buffer) != 0)
+		tst_res(TPASS, "mmap() functionality successful");
+	else
+		tst_res(TFAIL, "mmap() functionality failed");
+
+	memset(&addr[file_sz], 0, 3);
+	SAFE_MUNMAP(addr, page_sz);
 }
 
 static void cleanup(void)
 {
-	close(fildes);
-	free(dummy);
-	tst_rmdir();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+	if (dummy)
+		free(dummy);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_tmpdir = 1
+};
-- 
2.41.0


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

Re: [LTP] [PATCH 2/4] syscalls/mmap01: Rewrite the test using new LTP API

[Cyril Hrubis]

Hi!
> +	if (system(cmd_buffer) != 0)
> +		tst_res(TPASS, "mmap() functionality successful");
> +	else
> +		tst_res(TFAIL, "mmap() functionality failed");

This is quite ugly. Can we please instead for a child process and run a
function that reads the file with read() and checks the data?

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 3/4] syscalls/mmap02: Rewrite the test using new LTP API

[Avinesh Kumar]

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap02.c | 208 ++++++------------------
 1 file changed, 49 insertions(+), 159 deletions(-)

diff --git a/testcases/kernel/syscalls/mmap/mmap02.c b/testcases/kernel/syscalls/mmap/mmap02.c
index 566cc323a..210e4c6ec 100644
--- a/testcases/kernel/syscalls/mmap/mmap02.c
+++ b/testcases/kernel/syscalls/mmap/mmap02.c
@@ -1,186 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
- *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY;  without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *  07/2001 Ported by Wayne Boyer
+ * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
  */
 
-/*
- * Test Description:
- *  Call mmap() with prot parameter set to PROT_READ and with the file
- *  descriptor being open for read, to  map a file creating mapped memory
- *  with read access. The minimum file permissions should be 0444.
- *
- *  The call should succeed to create the mapped region with required
- *  attributes.
- *
- * Expected Result:
- *  mmap() should succeed returning the address of the mapped region,
- *  the mapped region should contain the contents of the mapped file.
+/*\
+ * [Description]
  *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
+ * Verify that, mmap() call with PROT_READ and a file descriptor which is
+ * open for read only, succeeds to map a file creating mapped memory with
+ * read access.
  */
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <string.h>
-#include <signal.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-
-#include "test.h"
 
-#define TEMPFILE	"mmapfile"
-
-char *TCID = "mmap02";
-int TST_TOTAL = 1;
+#include <stdlib.h>
+#include <stdio.h>
+#include "tst_test.h"
 
+#define TEMPFILE "mmapfile"
+static ssize_t page_sz;
+static int fd;
 static char *addr;
 static char *dummy;
-static size_t page_sz;
-static int fildes;
-
-static void setup(void);
-static void cleanup(void);
-
-int main(int ac, char **av)
-{
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/*
-		 * Call mmap to map the temporary file 'TEMPFILE'
-		 * with read access.
-		 */
-		errno = 0;
-		addr = mmap(0, page_sz, PROT_READ,
-			    MAP_FILE | MAP_SHARED, fildes, 0);
-
-		/* Check for the return value of mmap() */
-		if (addr == MAP_FAILED) {
-			tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE);
-			continue;
-		}
-
-		/*
-		 * Read the file contents into the dummy
-		 * string.
-		 */
-		if (read(fildes, dummy, page_sz) < 0) {
-			tst_brkm(TFAIL | TERRNO, cleanup,
-				 "reading %s failed", TEMPFILE);
-		}
-
-		/*
-		 * Check whether mapped memory region has
-		 * the file contents.
-		 */
-		if (memcmp(dummy, addr, page_sz)) {
-			tst_resm(TFAIL, "mapped memory area contains "
-				 "invalid data");
-		} else {
-			tst_resm(TPASS,
-				 "Functionality of mmap() successful");
-		}
-
-		/* Clean up things in case we are looping */
-		/* Unmap the mapped memory */
-		if (munmap(addr, page_sz) != 0) {
-			tst_brkm(TFAIL | TERRNO, cleanup, "munmapping failed");
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
 
 static void setup(void)
 {
-	char *tst_buff;
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	char *buf;
 
 	page_sz = getpagesize();
+	buf = SAFE_CALLOC(page_sz, sizeof(char));
+	memset(buf, 'A', page_sz);
 
-	/* Allocate space for the test buffer */
-	if ((tst_buff = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, NULL, "calloc failed (tst_buff)");
-	}
-
-	/* Fill the test buffer with the known data */
-	memset(tst_buff, 'A', page_sz);
+	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
+	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, page_sz);
+	free(buf);
+	SAFE_FCHMOD(fd, 0444);
+	SAFE_CLOSE(fd);
 
-	tst_tmpdir();
-
-	/* Creat a temporary file used for mapping */
-	if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) {
-		free(tst_buff);
-		tst_brkm(TFAIL | TERRNO, cleanup, "opening %s failed",
-			 TEMPFILE);
-	}
-
-	/* Write test buffer contents into temporary file */
-	if (write(fildes, tst_buff, page_sz) < (int)page_sz) {
-		free(tst_buff);
-		tst_brkm(TFAIL | TERRNO, cleanup,
-			 "writing to %s failed", TEMPFILE);
-	}
-
-	/* Free the memory allocated for test buffer */
-	free(tst_buff);
-
-	/* Change Mode permissions on Temporary file */
-	if (fchmod(fildes, 0444) < 0) {
-		tst_brkm(TFAIL | TERRNO, cleanup, "fchmod(%s, 0444) failed",
-			 TEMPFILE);
-	}
+	fd = SAFE_OPEN(TEMPFILE, O_RDONLY);
+	dummy = SAFE_CALLOC(page_sz, sizeof(char));
+}
 
-	/* Close the temporary file */
-	if (close(fildes) < 0) {
-		tst_brkm(TFAIL | TERRNO, cleanup, "closing %s failed",
-			 TEMPFILE);
+static void run(void)
+{
+	addr = mmap(0, page_sz, PROT_READ, MAP_FILE | MAP_SHARED, fd, 0);
+	if (addr == MAP_FAILED) {
+		tst_res(TFAIL | TERRNO, "mmap() of %s failed", TEMPFILE);
+		return;
 	}
 
-	/* Open the temporary file again, - Readonly mode */
-	if ((fildes = open(TEMPFILE, O_RDONLY)) < 0) {
-		tst_brkm(TFAIL, cleanup, "reopening %s readonly failed",
-			 TEMPFILE);
-	}
+	SAFE_READ(1, fd, dummy, page_sz);
+	SAFE_LSEEK(fd, 0, SEEK_SET);
 
-	/* Allocate and initialize dummy string of system page size bytes */
-	if ((dummy = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, cleanup, "calloc failed (dummy)");
-	}
+	if (memcmp(dummy, addr, page_sz) == 0)
+		tst_res(TPASS, "mmap() functionality successful");
+	else
+		tst_res(TFAIL, "mapped memory area contains invalid data");
 
+	SAFE_MUNMAP(addr, page_sz);
 }
 
 static void cleanup(void)
 {
-	close(fildes);
-	free(dummy);
-	tst_rmdir();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+	if (dummy)
+		free(dummy);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_tmpdir = 1
+};
-- 
2.41.0


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

Re: [LTP] [PATCH 3/4] syscalls/mmap02: Rewrite the test using new LTP API

[Cyril Hrubis]

Hi!
>  static void setup(void)
>  {
> -	char *tst_buff;
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> +	char *buf;
>  
>  	page_sz = getpagesize();
> +	buf = SAFE_CALLOC(page_sz, sizeof(char));
                                   ^
				   this is 1 by definition

So we can do just SAFE_MALLOC(page_sz);

> +	memset(buf, 'A', page_sz);
>  
> -	/* Allocate space for the test buffer */
> -	if ((tst_buff = calloc(page_sz, sizeof(char))) == NULL) {
> -		tst_brkm(TFAIL, NULL, "calloc failed (tst_buff)");
> -	}
> -
> -	/* Fill the test buffer with the known data */
> -	memset(tst_buff, 'A', page_sz);
> +	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
> +	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, page_sz);
> +	free(buf);
> +	SAFE_FCHMOD(fd, 0444);
> +	SAFE_CLOSE(fd);
>  
> -	tst_tmpdir();
> -
> -	/* Creat a temporary file used for mapping */
> -	if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) {
> -		free(tst_buff);
> -		tst_brkm(TFAIL | TERRNO, cleanup, "opening %s failed",
> -			 TEMPFILE);
> -	}
> -
> -	/* Write test buffer contents into temporary file */
> -	if (write(fildes, tst_buff, page_sz) < (int)page_sz) {
> -		free(tst_buff);
> -		tst_brkm(TFAIL | TERRNO, cleanup,
> -			 "writing to %s failed", TEMPFILE);
> -	}
> -
> -	/* Free the memory allocated for test buffer */
> -	free(tst_buff);
> -
> -	/* Change Mode permissions on Temporary file */
> -	if (fchmod(fildes, 0444) < 0) {
> -		tst_brkm(TFAIL | TERRNO, cleanup, "fchmod(%s, 0444) failed",
> -			 TEMPFILE);
> -	}
> +	fd = SAFE_OPEN(TEMPFILE, O_RDONLY);
> +	dummy = SAFE_CALLOC(page_sz, sizeof(char));

Why do we allocate second buffer? Cant we just keep the buf? Bonus point
is that the buf was alread filled with data we expect in the file, so we
don't have to read them with read() in the run() function.

> +}
>  
> -	/* Close the temporary file */
> -	if (close(fildes) < 0) {
> -		tst_brkm(TFAIL | TERRNO, cleanup, "closing %s failed",
> -			 TEMPFILE);
> +static void run(void)
> +{
> +	addr = mmap(0, page_sz, PROT_READ, MAP_FILE | MAP_SHARED, fd, 0);
> +	if (addr == MAP_FAILED) {
> +		tst_res(TFAIL | TERRNO, "mmap() of %s failed", TEMPFILE);
> +		return;
>  	}
>  
> -	/* Open the temporary file again, - Readonly mode */
> -	if ((fildes = open(TEMPFILE, O_RDONLY)) < 0) {
> -		tst_brkm(TFAIL, cleanup, "reopening %s readonly failed",
> -			 TEMPFILE);
> -	}
> +	SAFE_READ(1, fd, dummy, page_sz);
> +	SAFE_LSEEK(fd, 0, SEEK_SET);
>  
> -	/* Allocate and initialize dummy string of system page size bytes */
> -	if ((dummy = calloc(page_sz, sizeof(char))) == NULL) {
> -		tst_brkm(TFAIL, cleanup, "calloc failed (dummy)");
> -	}
> +	if (memcmp(dummy, addr, page_sz) == 0)
> +		tst_res(TPASS, "mmap() functionality successful");
> +	else
> +		tst_res(TFAIL, "mapped memory area contains invalid data");
>  
> +	SAFE_MUNMAP(addr, page_sz);
>  }
>  
>  static void cleanup(void)
>  {
> -	close(fildes);
> -	free(dummy);
> -	tst_rmdir();
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +	if (dummy)
> +		free(dummy);

free(NULL) is no-op so there is no need for the if ().

>  }
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run,
> +	.needs_tmpdir = 1
> +};
> -- 
> 2.41.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 4/4] syscalls/mmap04: Rewrite the test using new LTP API

[Avinesh Kumar]

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap04.c | 200 ++++++------------------
 1 file changed, 46 insertions(+), 154 deletions(-)

diff --git a/testcases/kernel/syscalls/mmap/mmap04.c b/testcases/kernel/syscalls/mmap/mmap04.c
index 43f7b7525..b818a5917 100644
--- a/testcases/kernel/syscalls/mmap/mmap04.c
+++ b/testcases/kernel/syscalls/mmap/mmap04.c
@@ -1,185 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
- *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY;  without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *  07/2001 Ported by Wayne Boyer
+ * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
  */
 
-/*
- * Test Description:
- *  Call mmap() to map a file creating a mapped region with read/exec access
- *  under the following conditions -
- *	- The prot parameter is set to PROT_READ|PROT_EXEC
- *	- The file descriptor is open for read
- *	- The file being mapped has read and execute permission bit set.
- *	- The minimum file permissions should be 0555.
- *
- *  The call should succeed to map the file creating mapped memory with the
- *  required attributes.
+/*\
+ * [Description]
  *
- * Expected Result:
- *  mmap() should succeed returning the address of the mapped region,
- *  and the mapped region should contain the contents of the mapped file.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
+ * Verify that, mmap() call with 'PROT_READ | PROT_EXEC; and file descriptor
+ * which is open for read only and has read and execute permission bits set,
+ * succeeds to map a file creating mapped memory with read/exec access.
  */
 
-#include <stdio.h>
 #include <stdlib.h>
-#include <sys/types.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <string.h>
-#include <signal.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-
-#include "test.h"
-
-#define TEMPFILE	"mmapfile"
-
-char *TCID = "mmap04";
-int TST_TOTAL = 1;
+#include "tst_test.h"
 
+#define TEMPFILE "mmapfile"
 static size_t page_sz;
+static int fd;
 static char *addr;
 static char *dummy;
-static int fildes;
-
-static void setup(void);
-static void cleanup(void);
-
-int main(int ac, char **av)
-{
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/*
-		 * Call mmap to map the temporary file 'TEMPFILE'
-		 * with read and execute access.
-		 */
-		errno = 0;
-		addr = mmap(0, page_sz, PROT_READ | PROT_EXEC,
-			    MAP_FILE | MAP_SHARED, fildes, 0);
-
-		/* Check for the return value of mmap() */
-		if (addr == MAP_FAILED) {
-			tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE);
-			continue;
-		}
-
-		/*
-		 * Read the file contents into the dummy
-		 * variable.
-		 */
-		if (read(fildes, dummy, page_sz) < 0) {
-			tst_brkm(TFAIL, cleanup, "reading %s failed",
-				 TEMPFILE);
-		}
-
-		/*
-		 * Check whether the mapped memory region
-		 * has the file contents.
-		 */
-		if (memcmp(dummy, addr, page_sz)) {
-			tst_resm(TFAIL,
-				 "mapped memory region contains invalid "
-				 "data");
-		} else {
-			tst_resm(TPASS,
-				 "Functionality of mmap() successful");
-		}
-
-		/* Clean up things in case we are looping. */
-		/* Unmap the mapped memory */
-		if (munmap(addr, page_sz) != 0) {
-			tst_brkm(TFAIL, cleanup, "munmapping failed");
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
 
 static void setup(void)
 {
-	char *tst_buff;
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	char *tst_buf;
 
 	page_sz = getpagesize();
+	tst_buf = SAFE_CALLOC(page_sz, sizeof(char));
+	memset(tst_buf, 'A', page_sz);
 
-	if ((tst_buff = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, NULL, "calloc failed (tst_buff)");
-	}
-
-	/* Fill the test buffer with the known data */
-	memset(tst_buff, 'A', page_sz);
+	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
+	SAFE_WRITE(SAFE_WRITE_ALL, fd, tst_buf, page_sz);
+	free(tst_buf);
 
-	tst_tmpdir();
+	SAFE_FCHMOD(fd, 0555);
+	SAFE_CLOSE(fd);
 
-	/* Creat a temporary file used for mapping */
-	if ((fildes = open(TEMPFILE, O_WRONLY | O_CREAT, 0666)) < 0) {
-		free(tst_buff);
-		tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE);
-	}
-
-	/* Write test buffer contents into temporary file */
-	if (write(fildes, tst_buff, page_sz) < (ssize_t)page_sz) {
-		free(tst_buff);
-		tst_brkm(TFAIL, cleanup, "writing to %s failed", TEMPFILE);
-	}
+	fd = SAFE_OPEN(TEMPFILE, O_RDONLY);
+	dummy = SAFE_CALLOC(page_sz, sizeof(char));
+}
 
-	/* Free the memory allocated for test buffer */
-	free(tst_buff);
+static void run(void)
+{
+	addr = mmap(0, page_sz, PROT_READ | PROT_EXEC, MAP_FILE | MAP_SHARED, fd, 0);
 
-	/* Make sure proper permissions set on file */
-	if (fchmod(fildes, 0555) < 0) {
-		tst_brkm(TFAIL, cleanup, "fchmod of %s failed", TEMPFILE);
+	if (addr == MAP_FAILED) {
+		tst_res(TFAIL | TERRNO, "mmap() of %s failed", TEMPFILE);
+		return;
 	}
 
-	/* Close the temporary file opened for write */
-	if (close(fildes) < 0) {
-		tst_brkm(TFAIL, cleanup, "closing %s failed", TEMPFILE);
-	}
+	SAFE_READ(1, fd, dummy, page_sz);
+	SAFE_LSEEK(fd, 0, SEEK_SET);
 
-	/* Allocate and initialize dummy string of system page size bytes */
-	if ((dummy = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, cleanup, "calloc failed (dummy)");
-	}
+	if (memcmp(dummy, addr, page_sz) == 0)
+		tst_res(TPASS, "mmap() functionality successful");
+	else
+		tst_res(TFAIL, "mapped memory region contains invalid data");
 
-	/* Open the temporary file again for reading */
-	if ((fildes = open(TEMPFILE, O_RDONLY)) < 0) {
-		tst_brkm(TFAIL, cleanup,
-			 "opening %s read-only failed", TEMPFILE);
-	}
+	SAFE_MUNMAP(addr, page_sz);
 }
 
 static void cleanup(void)
 {
-	close(fildes);
-	free(dummy);
-	tst_rmdir();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+	if (dummy)
+		free(dummy);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_tmpdir = 1
+};
-- 
2.41.0


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

Re: [LTP] [PATCH 4/4] syscalls/mmap04: Rewrite the test using new LTP API

[Cyril Hrubis]

Hi!
This is nearly identical to the mmap02.c and the same comments apply
here.

Also the test does not check the PROT_EXEC part at all. If we wanted to
properly test PROT_EXEC we would have to copy a function code to the
file first then execute it, something that mprotect04 does, but since
that flag is already tested in mprotect04 it does not make that much
sense to have it here as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 4/4] syscalls/mmap04: Rewrite the test using new LTP API

[Avinesh Kumar]

Hi Cyril,

On Tuesday, August 29, 2023 3:33:46 PM IST Cyril Hrubis wrote:
> Hi!
> This is nearly identical to the mmap02.c and the same comments apply
> here.
> 
> Also the test does not check the PROT_EXEC part at all. If we wanted to
> properly test PROT_EXEC we would have to copy a function code to the
> file first then execute it, something that mprotect04 does, but since
> that flag is already tested in mprotect04 it does not make that much
> sense to have it here as well.

Yes, I agree this test does not cover PROT_EXEC scenario. So, if this case is 
covered in mprotect04, do you recommend dropping this test altogether from 
here?

--
Regards,
Avinesh



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

Re: [LTP] [PATCH 4/4] syscalls/mmap04: Rewrite the test using new LTP API

[Cyril Hrubis]

Hi!
> > This is nearly identical to the mmap02.c and the same comments apply
> > here.
> > 
> > Also the test does not check the PROT_EXEC part at all. If we wanted to
> > properly test PROT_EXEC we would have to copy a function code to the
> > file first then execute it, something that mprotect04 does, but since
> > that flag is already tested in mprotect04 it does not make that much
> > sense to have it here as well.
> 
> Yes, I agree this test does not cover PROT_EXEC scenario. So, if this case is 
> covered in mprotect04, do you recommend dropping this test altogether from 
> here?

Testing that the flag is accepted by mmap() and mmap succeeds is
actually enough of test, that validates that the flag parsing in
sys_mmap() function in kernel works fine.

One option how to make the test more interesting would be to:

1) create mappings with different PROT_* flags
2) parse /proc/self/maps and check that the flags match what is there

That way we would validate that all valid flag combination are parsed
fine and printed correctly in /proc/self/maps.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH] syscalls/mmap04: Validate mapping perms in /proc/self/maps

[Avinesh Kumar]

Validating the various combinations of prot+flags arguments in mmap()
call and parsing the /proc/self/maps file to verifying resulting mapping
has the permission bits as requested in mmap() call.

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap04.c | 274 ++++++++----------------
 1 file changed, 89 insertions(+), 185 deletions(-)
 rewrite testcases/kernel/syscalls/mmap/mmap04.c (97%)

diff --git a/testcases/kernel/syscalls/mmap/mmap04.c b/testcases/kernel/syscalls/mmap/mmap04.c
dissimilarity index 97%
index 43f7b7525..a15dfedbe 100644
--- a/testcases/kernel/syscalls/mmap/mmap04.c
+++ b/testcases/kernel/syscalls/mmap/mmap04.c
@@ -1,185 +1,89 @@
-/*
- * Copyright (c) International Business Machines  Corp., 2001
- *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY;  without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * Test Description:
- *  Call mmap() to map a file creating a mapped region with read/exec access
- *  under the following conditions -
- *	- The prot parameter is set to PROT_READ|PROT_EXEC
- *	- The file descriptor is open for read
- *	- The file being mapped has read and execute permission bit set.
- *	- The minimum file permissions should be 0555.
- *
- *  The call should succeed to map the file creating mapped memory with the
- *  required attributes.
- *
- * Expected Result:
- *  mmap() should succeed returning the address of the mapped region,
- *  and the mapped region should contain the contents of the mapped file.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <string.h>
-#include <signal.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-
-#include "test.h"
-
-#define TEMPFILE	"mmapfile"
-
-char *TCID = "mmap04";
-int TST_TOTAL = 1;
-
-static size_t page_sz;
-static char *addr;
-static char *dummy;
-static int fildes;
-
-static void setup(void);
-static void cleanup(void);
-
-int main(int ac, char **av)
-{
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/*
-		 * Call mmap to map the temporary file 'TEMPFILE'
-		 * with read and execute access.
-		 */
-		errno = 0;
-		addr = mmap(0, page_sz, PROT_READ | PROT_EXEC,
-			    MAP_FILE | MAP_SHARED, fildes, 0);
-
-		/* Check for the return value of mmap() */
-		if (addr == MAP_FAILED) {
-			tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE);
-			continue;
-		}
-
-		/*
-		 * Read the file contents into the dummy
-		 * variable.
-		 */
-		if (read(fildes, dummy, page_sz) < 0) {
-			tst_brkm(TFAIL, cleanup, "reading %s failed",
-				 TEMPFILE);
-		}
-
-		/*
-		 * Check whether the mapped memory region
-		 * has the file contents.
-		 */
-		if (memcmp(dummy, addr, page_sz)) {
-			tst_resm(TFAIL,
-				 "mapped memory region contains invalid "
-				 "data");
-		} else {
-			tst_resm(TPASS,
-				 "Functionality of mmap() successful");
-		}
-
-		/* Clean up things in case we are looping. */
-		/* Unmap the mapped memory */
-		if (munmap(addr, page_sz) != 0) {
-			tst_brkm(TFAIL, cleanup, "munmapping failed");
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	char *tst_buff;
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	page_sz = getpagesize();
-
-	if ((tst_buff = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, NULL, "calloc failed (tst_buff)");
-	}
-
-	/* Fill the test buffer with the known data */
-	memset(tst_buff, 'A', page_sz);
-
-	tst_tmpdir();
-
-	/* Creat a temporary file used for mapping */
-	if ((fildes = open(TEMPFILE, O_WRONLY | O_CREAT, 0666)) < 0) {
-		free(tst_buff);
-		tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE);
-	}
-
-	/* Write test buffer contents into temporary file */
-	if (write(fildes, tst_buff, page_sz) < (ssize_t)page_sz) {
-		free(tst_buff);
-		tst_brkm(TFAIL, cleanup, "writing to %s failed", TEMPFILE);
-	}
-
-	/* Free the memory allocated for test buffer */
-	free(tst_buff);
-
-	/* Make sure proper permissions set on file */
-	if (fchmod(fildes, 0555) < 0) {
-		tst_brkm(TFAIL, cleanup, "fchmod of %s failed", TEMPFILE);
-	}
-
-	/* Close the temporary file opened for write */
-	if (close(fildes) < 0) {
-		tst_brkm(TFAIL, cleanup, "closing %s failed", TEMPFILE);
-	}
-
-	/* Allocate and initialize dummy string of system page size bytes */
-	if ((dummy = calloc(page_sz, sizeof(char))) == NULL) {
-		tst_brkm(TFAIL, cleanup, "calloc failed (dummy)");
-	}
-
-	/* Open the temporary file again for reading */
-	if ((fildes = open(TEMPFILE, O_RDONLY)) < 0) {
-		tst_brkm(TFAIL, cleanup,
-			 "opening %s read-only failed", TEMPFILE);
-	}
-}
-
-static void cleanup(void)
-{
-	close(fildes);
-	free(dummy);
-	tst_rmdir();
-}
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines  Corp., 2001
+ *  07/2001 Ported by Wayne Boyer
+ * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that, after a successful mmap() call, permission bits of the new
+ * mapping in /proc/pid/maps file matches the prot and flags arguments in
+ * mmap() call.
+ */
+
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+
+#define MMAPSIZE 1024
+static char *addr;
+
+static struct tcase {
+	int prot;
+	int flags;
+	char *exp_perms;
+} tcases[] = {
+	{PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, "---p"},
+	{PROT_NONE, MAP_ANONYMOUS | MAP_SHARED, "---s"},
+	{PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, "r--p"},
+	{PROT_READ, MAP_ANONYMOUS | MAP_SHARED, "r--s"},
+	{PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, "-w-p"},
+	{PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, "-w-s"},
+	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, "rw-p"},
+	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, "rw-s"},
+	{PROT_READ | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "r-xp"},
+	{PROT_READ | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "r-xs"},
+	{PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "-wxp"},
+	{PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "-wxs"},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "rwxp"},
+	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "rwxs"}
+};
+
+static void get_map_perms(char *addr_str, char *perms)
+{
+	FILE *file;
+	char line[BUFSIZ];
+
+	file = SAFE_FOPEN("/proc/self/maps", "r");
+
+	while (fgets(line, sizeof(line), file)) {
+		if (strstr(line, addr_str) != NULL) {
+			if (sscanf(line, "%*x-%*x %s", perms) != 1)
+				tst_brk(TBROK, "failed to find permission string in %s", line);
+			break;
+		}
+	}
+
+	SAFE_FCLOSE(file);
+	file = NULL;
+}
+
+static void run(unsigned int i)
+{
+	struct tcase *tc = &tcases[i];
+	char addr_str[20];
+	char perms[8];
+
+	addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0);
+
+	sprintf(addr_str, "%p", addr);
+	if (sscanf(addr_str, "0x%s", addr_str) != 1)
+		tst_brk(TBROK, "failed to find address string");
+
+	get_map_perms(addr_str, perms);
+
+	if (!strcmp(perms, tc->exp_perms))
+		tst_res(TPASS, "mapping permissions in /proc matched: %s", perms);
+	else
+		tst_res(TFAIL, "mapping permissions in /proc mismatched,"
+						" expected: %s, found: %s",
+						tc->exp_perms, perms);
+
+	SAFE_MUNMAP(addr, MMAPSIZE);
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+};
-- 
2.41.0


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

Re: [LTP] [PATCH] syscalls/mmap04: Validate mapping perms in /proc/self/maps

[Cyril Hrubis]

Hi!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) International Business Machines  Corp., 2001
> + *  07/2001 Ported by Wayne Boyer
> + * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that, after a successful mmap() call, permission bits of the new
> + * mapping in /proc/pid/maps file matches the prot and flags arguments in
> + * mmap() call.
> + */
> +
> +#include "tst_test.h"
> +#include "tst_safe_stdio.h"
> +
> +#define MMAPSIZE 1024
> +static char *addr;
> +
> +static struct tcase {
> +	int prot;
> +	int flags;
> +	char *exp_perms;
> +} tcases[] = {
> +	{PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, "---p"},
> +	{PROT_NONE, MAP_ANONYMOUS | MAP_SHARED, "---s"},
> +	{PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, "r--p"},
> +	{PROT_READ, MAP_ANONYMOUS | MAP_SHARED, "r--s"},
> +	{PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, "-w-p"},
> +	{PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, "-w-s"},
> +	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, "rw-p"},
> +	{PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, "rw-s"},
> +	{PROT_READ | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "r-xp"},
> +	{PROT_READ | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "r-xs"},
> +	{PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "-wxp"},
> +	{PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "-wxs"},
> +	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, "rwxp"},
> +	{PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, "rwxs"}
> +};
> +
> +static void get_map_perms(char *addr_str, char *perms)
> +{
> +	FILE *file;
> +	char line[BUFSIZ];
> +
> +	file = SAFE_FOPEN("/proc/self/maps", "r");
> +
> +	while (fgets(line, sizeof(line), file)) {
> +		if (strstr(line, addr_str) != NULL) {
> +			if (sscanf(line, "%*x-%*x %s", perms) != 1)
> +				tst_brk(TBROK, "failed to find permission string in %s", line);
> +			break;
> +		}
> +	}
> +
> +	SAFE_FCLOSE(file);
> +	file = NULL;

No need to set the file to NULL here, it's on stack and will perish once
we leave this function.

Rather than that we should check that the perms was actually set at this
point. It's unlikely that it will be missing, but we are writing kernel
tests so we should really make sure we actually found it.

Also I do wonder if we can use SAFE_FILE_LINES_SCANF() here that would
sove the the problem since it will TBROK if the line wasn't found. For
that we would have to snprintf() the fmt string with the address so that
we pass something as "5560620ce000-%-x %s" to the scanf.

> +}
> +
> +static void run(unsigned int i)
> +{
> +	struct tcase *tc = &tcases[i];
> +	char addr_str[20];
> +	char perms[8];
> +
> +	addr = SAFE_MMAP(NULL, MMAPSIZE, tc->prot, tc->flags, -1, 0);
> +
> +	sprintf(addr_str, "%p", addr);
> +	if (sscanf(addr_str, "0x%s", addr_str) != 1)
> +		tst_brk(TBROK, "failed to find address string");

I guess that casting the address to (uintptr_t) and printing it with
"%"PRIxPTR is cleaner solution.

> +	get_map_perms(addr_str, perms);
> +
> +	if (!strcmp(perms, tc->exp_perms))
> +		tst_res(TPASS, "mapping permissions in /proc matched: %s", perms);
> +	else
> +		tst_res(TFAIL, "mapping permissions in /proc mismatched,"
> +						" expected: %s, found: %s",
> +						tc->exp_perms, perms);
> +
> +	SAFE_MUNMAP(addr, MMAPSIZE);
> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
> +};
> -- 
> 2.41.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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