diff mbox series

[v2,1/2] syscalls/mmap01: Rewrite the test using new LTP API

Message ID 20230901124816.30437-1-akumar@suse.de
State Superseded
Headers show
Series [v2,1/2] syscalls/mmap01: Rewrite the test using new LTP API | expand

Commit Message

Avinesh Kumar Sept. 1, 2023, 12:48 p.m. UTC 
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 | 221 ++++++------------------
 1 file changed, 57 insertions(+), 164 deletions(-)

Comments

Cyril Hrubis Sept. 11, 2023, 9:52 a.m. UTC | #1 
Hi!
> +static void run(void)
> +{
> +	char buf[20];
>  
> -	/* 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");
> -	}
> +	addr = SAFE_MMAP(NULL, page_sz, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0);
>  
> -	/* 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);
> -	}
> +	if (memcmp(&addr[file_sz], dummy, page_sz - file_sz) != 0)
> +		tst_brk(TFAIL, "mapped memory area contains invalid data");

The tst_brk() does not work correctly with TFAIL, I suppose that the
best we can do here is to tst_res(TFAIL, ...) and goto SAFE_UNMAP().

> -	/* 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);
> -	}
> +	addr[file_sz] = 'X';
> +	addr[file_sz + 1] = 'Y';
> +	addr[file_sz + 2] = 'Z';
>  
> -	/* Get the size of temporary file */
> -	if (stat(TEMPFILE, &stat_buf) < 0) {
> -		tst_brkm(TFAIL | TERRNO, cleanup, "stat of %s failed",
> -			 TEMPFILE);
> -	}
> -	file_sz = stat_buf.st_size;
> +	if (msync(addr, page_sz, MS_SYNC) != 0)
> +		tst_brk(TFAIL | TERRNO, "failed to sync mapped file");

Here as well. Or alternatively we can add SAFE_MSYNC() to the test
library and use that.

> -	page_sz = getpagesize();
> +	SAFE_READ(0, fd, buf, sizeof(buf));
> +	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 (strstr(buf, "XYZ") == NULL)
> +		tst_res(TPASS, "mmap() functionality successful");
> +	else
> +		tst_res(TFAIL, "mmap() functionality failed");

This can possibly crash, there is no guarantee that the buf is zero
terminated to be suitable for str*() functions.

I guess that the easiest solution would be memchr() for each of the
letters. However the first thing that we should check is actually the
return value from the read(), if that matches the number of bytes we
wrote to the buffer we can actually be sure that we haven't written
anything past the data that did exist in the file.

> -	/* Create the command which will be executed in the test */
> -	sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", Path_name, TEMPFILE);
> +	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);

Again, free(NULL) is no-op, no need for the if here.

>  }
> +
> +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
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/mmap/mmap01.c b/testcases/kernel/syscalls/mmap/mmap01.c
index 99266b57f..4ec38639e 100644
--- a/testcases/kernel/syscalls/mmap/mmap01.c
+++ b/testcases/kernel/syscalls/mmap/mmap01.c
@@ -1,194 +1,87 @@ 
+// 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.
- *
- * 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.
+/*\
+ * [Description]
  *
- * 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"
 
-#define TEMPFILE	"mmapfile"
-
-char *TCID = "mmap01";
-int TST_TOTAL = 1;
+#include <stdlib.h>
+#include "tst_test.h"
 
-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 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 char *dummy;
+static char *addr;
 
 static void setup(void)
 {
 	struct stat stat_buf;
-	char Path_name[PATH_MAX];
 	char write_buf[] = "hello world\n";
 
-	tst_sig(FORK, DEF_HANDLER, cleanup);
+	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
+
+	SAFE_WRITE(SAFE_WRITE_ALL, fd, write_buf, strlen(write_buf));
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+	SAFE_STAT(TEMPFILE, &stat_buf);
+
+	file_sz = stat_buf.st_size;
+	page_sz = getpagesize();
 
-	TEST_PAUSE;
+	dummy = SAFE_MALLOC(page_sz);
+	memset(dummy, 0, page_sz);
+}
 
-	tst_tmpdir();
+static void run(void)
+{
+	char buf[20];
 
-	/* 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");
-	}
+	addr = SAFE_MMAP(NULL, page_sz, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0);
 
-	/* 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);
-	}
+	if (memcmp(&addr[file_sz], dummy, page_sz - file_sz) != 0)
+		tst_brk(TFAIL, "mapped memory area contains invalid data");
 
-	/* 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);
-	}
+	addr[file_sz] = 'X';
+	addr[file_sz + 1] = 'Y';
+	addr[file_sz + 2] = 'Z';
 
-	/* Get the size of temporary file */
-	if (stat(TEMPFILE, &stat_buf) < 0) {
-		tst_brkm(TFAIL | TERRNO, cleanup, "stat of %s failed",
-			 TEMPFILE);
-	}
-	file_sz = stat_buf.st_size;
+	if (msync(addr, page_sz, MS_SYNC) != 0)
+		tst_brk(TFAIL | TERRNO, "failed to sync mapped file");
 
-	page_sz = getpagesize();
+	SAFE_READ(0, fd, buf, sizeof(buf));
+	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 (strstr(buf, "XYZ") == NULL)
+		tst_res(TPASS, "mmap() functionality successful");
+	else
+		tst_res(TFAIL, "mmap() functionality failed");
 
-	/* Create the command which will be executed in the test */
-	sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", Path_name, TEMPFILE);
+	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
+};