diff mbox series

[2/5] syscalls/mmap09: Rewrite the test using new LTP API

Message ID 20230825063932.30875-2-akumar@suse.de
State Changes Requested
Headers show
Series [1/5] syscalls/mmap08: Rewrite the test using new LTP API | expand

Commit Message

Avinesh Kumar Aug. 25, 2023, 6:38 a.m. UTC
Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap09.c | 134 ++++++++----------------
 1 file changed, 45 insertions(+), 89 deletions(-)

Comments

Richard Palethorpe Sept. 1, 2023, 8:27 a.m. UTC | #1
Hello,

Avinesh Kumar <akumar@suse.de> writes:

> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>  testcases/kernel/syscalls/mmap/mmap09.c | 134 ++++++++----------------
>  1 file changed, 45 insertions(+), 89 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mmap/mmap09.c b/testcases/kernel/syscalls/mmap/mmap09.c
> index 4ab0da470..6f59baf4a 100644
> --- a/testcases/kernel/syscalls/mmap/mmap09.c
> +++ b/testcases/kernel/syscalls/mmap/mmap09.c
> @@ -1,119 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * Copyright (c) International Business Machines  Corp., 2003
> - *
> - * 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
> + * Copyright (c) International Business Machines  Corp., 2001
> + *  04/2003 Written by Paul Larson
> + * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
>   */
>  
> -/*
> - * Test Description:
> - *  Verify that truncating a mmaped file works correctly.
> - *
> - * Expected Result:
> - *  ftruncate should be allowed to increase, decrease, or zero the
> - *  size of a file that has been mmaped
> +/*\
> + * [Description]
>   *
> - *  Test:
> - *   Use ftruncate to shrink the file while it is mapped
> - *   Use ftruncate to grow the file while it is mapped
> - *   Use ftruncate to zero the size of the file while it is mapped
> - *
> - * HISTORY
> - *	04/2003 Written by Paul Larson
> + * Verify that, once we have a file mapping created using mmap(), we can
> + * successfully shrink, grow or zero the size of the mapped file using
> + * ftruncate.
>   */
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <fcntl.h>
> -#include <errno.h>
> -#include <sys/mman.h>
> -#include <sys/types.h>
> -#include "test.h"
>  
> -#define mapsize (1 << 14)
>  
> -char *TCID = "mmap09";
> -int TST_TOTAL = 3;
> +#include <stdlib.h>
> +#include "tst_test.h"
>  
> +#define mapsize (1 << 14)
> +#define TEMPFILE "mmapfile"
>  static int fd;
> -static char *maddr;
> +static char *addr;
>  
> -static struct test_case_t {
> +static struct tcase {
>  	off_t newsize;
>  	char *desc;
> -} TC[] = {
> -	{mapsize - 8192, "ftruncate mmaped file to a smaller size"},
> -	{mapsize + 1024, "ftruncate mmaped file to a larger size"},
> -	{0, "ftruncate mmaped file to 0 size"},
> +} tcases[] = {
> +	{mapsize - 8192, "ftruncate mapped file to a smaller size"},
> +	{mapsize + 1024, "ftruncate mapped file to a bigger size"},
> +	{0, "ftruncate mapped file to zero size"}
>  };
>  
> -static void setup(void);
> -static void cleanup(void);
> -
> -int main(int argc, char **argv)
> +static void setup(void)
>  {
> -	int lc;
> -	int i;
> -
> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	setup();
> +	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
>  
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -		for (i = 0; i < TST_TOTAL; i++) {
> -			TEST(ftruncate(fd, TC[i].newsize));
> +	SAFE_FTRUNCATE(fd, mapsize);
>  
> -			if (TEST_RETURN == -1) {
> -				tst_resm(TFAIL | TTERRNO, "%s", TC[i].desc);
> -			} else {
> -				tst_resm(TPASS, "%s", TC[i].desc);
> -			}
> -		}
> +	addr = mmap(0, mapsize, PROT_READ | PROT_WRITE, MAP_FILE |
> MAP_SHARED, fd, 0);

Why don't we use SAFE_MMAP?

> +	if (addr == MAP_FAILED)
> +		tst_brk(TFAIL | TERRNO, "mmap() failed");
>  
> -	}
> +	memset(addr, 'A', mapsize);
>  
> -	cleanup();
> -	tst_exit();
>  }
>  
> -static void setup(void)
> +static void run(unsigned int i)
>  {
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> +	struct stat stat_buf;
> +	struct tcase *tc = &tcases[i];
>  
> -	tst_tmpdir();
> +	TST_EXP_PASS(ftruncate(fd, tc->newsize), "%s", tc->desc);
>  
> -	if ((fd = open("mmaptest", O_RDWR | O_CREAT, 0666)) < 0)
> -		tst_brkm(TFAIL | TERRNO, cleanup, "opening mmaptest failed");
> +	SAFE_FSTAT(fd, &stat_buf);
> +	TST_EXP_EQ_LI(stat_buf.st_size, tc->newsize);
>  
> -	/* ftruncate the file to 16k */
> -	if (ftruncate(fd, mapsize) < 0)
> -		tst_brkm(TFAIL | TERRNO, cleanup, "ftruncate file failed");
> -
> -	maddr = mmap(0, mapsize, PROT_READ | PROT_WRITE,
> -		     MAP_FILE | MAP_SHARED, fd, 0);
> -	if (maddr == MAP_FAILED)
> -		tst_brkm(TFAIL | TERRNO, cleanup, "mmapping mmaptest failed");
> -
> -	/* fill up the file with A's */
> -	memset(maddr, 'A', mapsize);
> +	SAFE_FTRUNCATE(fd, mapsize);
>  }
>  
>  static void cleanup(void)
>  {
> -	munmap(maddr, mapsize);
> -	close(fd);
> -	tst_rmdir();
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +	if (addr)
> +		SAFE_MUNMAP(addr, mapsize);
>  }
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.needs_tmpdir = 1

Can we use all file systems?

The test is mapping a file and performing an operation on it. So this is
basically a file system test.

BTW this test seems weak. I don't know what truncating the file without
then trying to access the newly OOB memory achieves. However it's what
the original test did, so it's up to you if you want to change anything.

Setting to changes requested
Cyril Hrubis Sept. 1, 2023, 9:04 a.m. UTC | #2
Hi!
> > +	addr = mmap(0, mapsize, PROT_READ | PROT_WRITE, MAP_FILE |
> > MAP_SHARED, fd, 0);
> 
> Why don't we use SAFE_MMAP?

I guess mainly because that would produce TBROK instead of TFAIL.

> Can we use all file systems?
> 
> The test is mapping a file and performing an operation on it. So this is
> basically a file system test.
> 
> BTW this test seems weak. I don't know what truncating the file without
> then trying to access the newly OOB memory achieves. However it's what
> the original test did, so it's up to you if you want to change anything.

I would vote for adding additional checks like this. I suppose that you
will get SIGBUS when accessing pages beyond new file size, so we should
probably fork a child, let it touch the truncated part of the file, and
check that it was killed by SIGBUS.
Richard Palethorpe Sept. 1, 2023, 9:11 a.m. UTC | #3
Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > +	addr = mmap(0, mapsize, PROT_READ | PROT_WRITE, MAP_FILE |
>> > MAP_SHARED, fd, 0);
>> 
>> Why don't we use SAFE_MMAP?
>
> I guess mainly because that would produce TBROK instead of TFAIL.
>
>> Can we use all file systems?
>> 
>> The test is mapping a file and performing an operation on it. So this is
>> basically a file system test.
>> 
>> BTW this test seems weak. I don't know what truncating the file without
>> then trying to access the newly OOB memory achieves. However it's what
>> the original test did, so it's up to you if you want to change anything.
>
> I would vote for adding additional checks like this. I suppose that you
> will get SIGBUS when accessing pages beyond new file size, so we should
> probably fork a child, let it touch the truncated part of the file, and
> check that it was killed by SIGBUS.

There is some overlap with mmap13 because that does check for
SIGBUS. Possibly these could be combined?
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/mmap/mmap09.c b/testcases/kernel/syscalls/mmap/mmap09.c
index 4ab0da470..6f59baf4a 100644
--- a/testcases/kernel/syscalls/mmap/mmap09.c
+++ b/testcases/kernel/syscalls/mmap/mmap09.c
@@ -1,119 +1,75 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) International Business Machines  Corp., 2003
- *
- * 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
+ * Copyright (c) International Business Machines  Corp., 2001
+ *  04/2003 Written by Paul Larson
+ * Copyright (c) 2023 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
  */
 
-/*
- * Test Description:
- *  Verify that truncating a mmaped file works correctly.
- *
- * Expected Result:
- *  ftruncate should be allowed to increase, decrease, or zero the
- *  size of a file that has been mmaped
+/*\
+ * [Description]
  *
- *  Test:
- *   Use ftruncate to shrink the file while it is mapped
- *   Use ftruncate to grow the file while it is mapped
- *   Use ftruncate to zero the size of the file while it is mapped
- *
- * HISTORY
- *	04/2003 Written by Paul Larson
+ * Verify that, once we have a file mapping created using mmap(), we can
+ * successfully shrink, grow or zero the size of the mapped file using
+ * ftruncate.
  */
-#include <stdio.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <sys/mman.h>
-#include <sys/types.h>
-#include "test.h"
 
-#define mapsize (1 << 14)
 
-char *TCID = "mmap09";
-int TST_TOTAL = 3;
+#include <stdlib.h>
+#include "tst_test.h"
 
+#define mapsize (1 << 14)
+#define TEMPFILE "mmapfile"
 static int fd;
-static char *maddr;
+static char *addr;
 
-static struct test_case_t {
+static struct tcase {
 	off_t newsize;
 	char *desc;
-} TC[] = {
-	{mapsize - 8192, "ftruncate mmaped file to a smaller size"},
-	{mapsize + 1024, "ftruncate mmaped file to a larger size"},
-	{0, "ftruncate mmaped file to 0 size"},
+} tcases[] = {
+	{mapsize - 8192, "ftruncate mapped file to a smaller size"},
+	{mapsize + 1024, "ftruncate mapped file to a bigger size"},
+	{0, "ftruncate mapped file to zero size"}
 };
 
-static void setup(void);
-static void cleanup(void);
-
-int main(int argc, char **argv)
+static void setup(void)
 {
-	int lc;
-	int i;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
+	fd = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666);
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(ftruncate(fd, TC[i].newsize));
+	SAFE_FTRUNCATE(fd, mapsize);
 
-			if (TEST_RETURN == -1) {
-				tst_resm(TFAIL | TTERRNO, "%s", TC[i].desc);
-			} else {
-				tst_resm(TPASS, "%s", TC[i].desc);
-			}
-		}
+	addr = mmap(0, mapsize, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0);
+	if (addr == MAP_FAILED)
+		tst_brk(TFAIL | TERRNO, "mmap() failed");
 
-	}
+	memset(addr, 'A', mapsize);
 
-	cleanup();
-	tst_exit();
 }
 
-static void setup(void)
+static void run(unsigned int i)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	struct stat stat_buf;
+	struct tcase *tc = &tcases[i];
 
-	tst_tmpdir();
+	TST_EXP_PASS(ftruncate(fd, tc->newsize), "%s", tc->desc);
 
-	if ((fd = open("mmaptest", O_RDWR | O_CREAT, 0666)) < 0)
-		tst_brkm(TFAIL | TERRNO, cleanup, "opening mmaptest failed");
+	SAFE_FSTAT(fd, &stat_buf);
+	TST_EXP_EQ_LI(stat_buf.st_size, tc->newsize);
 
-	/* ftruncate the file to 16k */
-	if (ftruncate(fd, mapsize) < 0)
-		tst_brkm(TFAIL | TERRNO, cleanup, "ftruncate file failed");
-
-	maddr = mmap(0, mapsize, PROT_READ | PROT_WRITE,
-		     MAP_FILE | MAP_SHARED, fd, 0);
-	if (maddr == MAP_FAILED)
-		tst_brkm(TFAIL | TERRNO, cleanup, "mmapping mmaptest failed");
-
-	/* fill up the file with A's */
-	memset(maddr, 'A', mapsize);
+	SAFE_FTRUNCATE(fd, mapsize);
 }
 
 static void cleanup(void)
 {
-	munmap(maddr, mapsize);
-	close(fd);
-	tst_rmdir();
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+	if (addr)
+		SAFE_MUNMAP(addr, mapsize);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_tmpdir = 1
+};