Re: [LTP] [PATCH 3/3] syscalls: munmap03: Convert to new API

["Ricardo B. Marlière via ltp" <ltp@lists.linux.it>]

On Fri Jul 4, 2025 at 9:14 AM -03, Andrea Cervesato wrote:
> Hi!
>
> On 7/3/25 8:39 PM, Ricardo B. Marlière via ltp wrote:
>> From: Ricardo B. Marlière <rbm@suse.com>
>>
>> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
>> ---
>>   testcases/kernel/syscalls/munmap/munmap03.c | 162 ++++++++--------------------
>>   1 file changed, 43 insertions(+), 119 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/munmap/munmap03.c b/testcases/kernel/syscalls/munmap/munmap03.c
>> index 60bcb93b0e359b579294d6745e1e729340c0d92e..7aab587ad4ca0e0add973a7344a338240f485647 100644
>> --- a/testcases/kernel/syscalls/munmap/munmap03.c
>> +++ b/testcases/kernel/syscalls/munmap/munmap03.c
>> @@ -1,148 +1,72 @@
>> +// 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
>> + * Copyright (c) International Business Machines  Corp., 2001
>> + *	07/2001 Ported by Wayne Boyer
>> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
>>    */
>>   
>> -/*
>> - * Test Description:
>> - *  Verify that,
>> - *   1. munmap() fails with -1 return value and sets errno to EINVAL
>> - *      if addresses in the range [addr,addr+len) are outside the valid
>> - *	range for the address space of a process.
>> - *   2. munmap() fails with -1 return value and sets errno to EINVAL
>> - *      if the len argument is 0.
>> - *   3. munmap() fails with -1 return value and sets errno to EINVAL
>> - *      if the addr argument is not a multiple of the page size as
>> - *      returned by sysconf().
>> +/*\
>> + * Verify that, munmap() fails with errno:
>>    *
>> - * HISTORY
>> - *	07/2001 Ported by Wayne Boyer
>> + * - EINVAL, if addresses in the range [addr,addr+len) are outside the valid
>> + *   range for the address space of a process.
>> + * - EINVAL, if the len argument is 0.
>> + * - EINVAL, if the addr argument is not a multiple of the page size as
>> + *   returned by sysconf().
>>    */
>>   
>> -#include <errno.h>
>> -#include <unistd.h>
>> -#include <fcntl.h>
>> -#include <sys/mman.h>
>> -#include <sys/resource.h>
>> -#include <sys/stat.h>
>> -
>> -#include "test.h"
>> -#include "safe_macros.h"
>> -
>> -char *TCID = "munmap03";
>> +#include "tst_test.h"
>>   
>>   static size_t page_sz;
>>   static char *global_addr;
>>   static size_t global_maplen;
>>   
>> -static void setup(void);
>> -static void cleanup(void);
>> +static struct tcase {
>> +	int exp_errno;
>> +	char *addr;
>> +	int len;
>>   
>> -static void test_einval1(void);
>> -static void test_einval2(void);
>> -static void test_einval3(void);
>> -static void (*testfunc[])(void) = { test_einval1, test_einval2, test_einval3 };
>> -int TST_TOTAL = ARRAY_SIZE(testfunc);
>> +} tcases[] = {
>> +	{ EINVAL, NULL, 0 },
>> +	{ EINVAL, NULL, 0 },
>> +	{ EINVAL, NULL, 0 },
>> +};
>>   
>> -int main(int ac, char **av)
>> +static void run(unsigned int i)
>>   {
>> -	int i, lc;
>> +	struct tcase *tc = &tcases[i];
>>   
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		for (i = 0; i < TST_TOTAL; i++)
>> -			(*testfunc[i])();
>> -	}
>> -
>> -	cleanup();
>> -	tst_exit();
>> +	TST_EXP_FAIL(munmap(tc->addr, tc->len), EINVAL);
>>   }
>>   
>>   static void setup(void)
>> -{
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -
>> -	TEST_PAUSE;
>> -
>> -	page_sz = (size_t)sysconf(_SC_PAGESIZE);
>> -
>> -	global_maplen = page_sz * 2;
>> -	global_addr = SAFE_MMAP(cleanup, NULL, global_maplen, PROT_READ |
>> -				PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> -}
>> -
>> -static void check_and_print(int expected_errno)
>> -{
>> -	if (TEST_RETURN == -1) {
>> -		if (TEST_ERRNO == expected_errno) {
>> -			tst_resm(TPASS | TTERRNO, "failed as expected");
>> -		} else {
>> -			tst_resm(TFAIL | TTERRNO,
>> -				 "failed unexpectedly; expected - %d : %s",
>> -				 expected_errno, strerror(expected_errno));
>> -		}
>> -	} else {
>> -		tst_resm(TFAIL, "munmap succeeded unexpectedly");
>> -	}
>> -}
>> -
>> -static void test_einval1(void)
>>   {
>>   	struct rlimit brkval;
>> -	char *addr;
>> -	size_t map_len;
>> -
>> -	SAFE_GETRLIMIT(cleanup, RLIMIT_DATA, &brkval);
>> -
>> -	addr = (char *)brkval.rlim_max;
>> -	map_len = page_sz * 2;
>> -
>> -	TEST(munmap(addr, map_len));
>>   
>> -	check_and_print(EINVAL);
>> -}
>> -
>> -static void test_einval2(void)
>> -{
>> -	char *addr = global_addr;
>> -	size_t map_len = 0;
>> -
>> -	TEST(munmap(addr, map_len));
>> -
>> -	check_and_print(EINVAL);
>> -}
>> +	page_sz = getpagesize();
>> +	global_maplen = page_sz * 2;
>> +	global_addr = SAFE_MMAP(NULL, global_maplen, PROT_READ | PROT_WRITE,
>> +				MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>   
>> -static void test_einval3(void)
>> -{
>> -	char *addr = (char *)(global_addr + 1);
>> -	size_t map_len = page_sz;
>> +	SAFE_GETRLIMIT(RLIMIT_DATA, &brkval);
>> +	tcases[0].addr = (char *)brkval.rlim_max;
>> +	tcases[0].len = page_sz * 2;
>>   
>> -	TEST(munmap(addr, map_len));
>> +	tcases[1].addr = global_addr;
>> +	tcases[1].len = 0;
>>   
>> -	check_and_print(EINVAL);
>> +	tcases[2].addr = (char *)(global_addr + 1);
>> +	tcases[2].len = page_sz;
> We don't usually use this approach because it's error prone. Instead, we 
> create different static variables and we reference them to the structure 
> after setting them inside setup(). For an example, you can take a look 
> at lsm_set_self_attr01.c .

Ack.

>>   }
>>   
>>   static void cleanup(void)
>>   {
>> -	if (munmap(global_addr, global_maplen) == -1)
>> -		tst_resm(TWARN | TERRNO, "munmap failed");
>> +	SAFE_MUNMAP(global_addr, global_maplen);
>>   }
>> +
>> +static struct tst_test test = {
>> +	.test = run,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +};
>>
> We are still missing many other errors which could be tested. Please 
> take a look at "man munmap" ERRORS section and try to verify how many of 
> them could be actually be tested in this code by passing invalid data.

Most of those are for mmap. The only one left seems to be ENOMEM, which
I don't think it's viable to test. WDYT?

>
> - Andrea


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