Re: [LTP] [PATCH] [PATCH v3] Migrating the libhugetlbfs/testcases/truncate_above_4GB.c test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Pavithra <pavrampu@linux.vnet.ibm.com>
> ---
> Changes in v3:
> - Fixed Warnings
> ---
>  runtest/hugetlb                               |   1 +
>  testcases/kernel/mem/.gitignore               |   1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap36.c  | 170 ++++++++++++++++++
>  3 files changed, 172 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap36.c
> 
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 0896d3c94..bd40a7a30 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -36,6 +36,7 @@ hugemmap30 hugemmap30
>  hugemmap31 hugemmap31
>  hugemmap32 hugemmap32
>  hugemmap34 hugemmap34
> +hugemmap36 hugemmap36
>  hugemmap05_1 hugemmap05 -m
>  hugemmap05_2 hugemmap05 -s
>  hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index b4455de51..2ddef6bf1 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -36,6 +36,7 @@
>  /hugetlb/hugemmap/hugemmap31
>  /hugetlb/hugemmap/hugemmap32
>  /hugetlb/hugemmap/hugemmap34
> +/hugetlb/hugemmap/hugemmap36
>  /hugetlb/hugeshmat/hugeshmat01
>  /hugetlb/hugeshmat/hugeshmat02
>  /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap36.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap36.c
> new file mode 100644
> index 000000000..a575d601e
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap36.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> + * Copyright (C) 2006 Hugh Dickins <hugh@veritas.com>
> + */
> +
> +/*\
> + *[Descripiton]
> + *
> + * At one stage, a misconversion of hugetlb_vmtruncate_list to a
> + * prio_tree meant that on 32-bit machines, truncates at or above 4GB
> + * could truncate lower pages, resulting in BUG_ON()s.
> + *
> + * WARNING: The offsets and addresses used within are specifically
> + * calculated to trigger the bug as it existed.  Don't mess with them
> + * unless you *really* know what you're doing.
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#define _LARGEFILE64_SOURCE
> +#define FOURGIG ((off64_t)0x100000000ULL)
> +#define MNTPOINT "hugetlbfs/"
> +
> +#include <signal.h>
> +#include <setjmp.h>
> +#include "hugetlb.h"
> +
> +static int page_size;
> +static long hpage_size;
> +static int fd = -1;
> +static volatile int test_pass;
> +static int err;
> +static int sigbus_count;
> +static sigjmp_buf sig_escape;
> +
> +static void sigbus_handler_fail(void)
> +{
> +	siglongjmp(sig_escape, 17);
> +}
> +
> +static void sigbus_handler_pass(void)
> +{
> +	test_pass = 1;
> +	siglongjmp(sig_escape, 17);
> +}
> +
> +static void run_test(void)
> +{
> +	static long long buggy_offset, truncate_point;
> +	void *p, *q;
> +	volatile unsigned int *pi, *qi;
> +
> +	struct sigaction sa_pass = {
> +		.sa_sigaction = sigbus_handler_pass,
> +		.sa_flags = SA_SIGINFO,
> +	};
> +
> +	struct sigaction sa_fail = {
> +    		.sa_sigaction = sigbus_handler_fail,
> +    		.sa_flags = SA_SIGINFO,
> +	};

There are wrong whitespaces here. Have you done 'make check' in the test
directory to find common problems?

> +	sigbus_count = 0;
> +	test_pass = 0;
> +
> +	buggy_offset = truncate_point / (hpage_size / page_size);
> +	buggy_offset = (long long)PALIGN(buggy_offset, hpage_size);
> +
> +	/* First get arena of three hpages size, at file offset 4GB */
> +	q = mmap64(NULL, 3*hpage_size, PROT_READ|PROT_WRITE,
> +		 MAP_PRIVATE, fd, truncate_point);
> +	if (q == MAP_FAILED)
> +		tst_brk(TBROK, "mmap() offset 4GB: %s", strerror(errno));

You shouldn't call mmap64() directly. WIth LFS enabled the 64bit variant
is choosen automatically and this can be SAFE_MMAP().

> +	qi = q;
> +	/* Touch the high page */
> +	*qi = 0;
> +
> +	/* This part of the test makes the problem more obvious, but
> +	 * is not essential.  It can't be done on segmented powerpc, where
> +	 * segment restrictions prohibit us from performing such a
> +	 * mapping, so skip it there. Similarly, ia64's address space
> +	 * restrictions prevent this.
> +	 */
> +#if (defined(__powerpc__) && defined(PPC_NO_SEGMENTS)) \
> +	|| !defined(__powerpc__) && !defined(__powerpc64__) \
> +	&& !defined(__ia64__)
> +	/* Replace middle hpage by tinypage mapping to trigger
> +	 * nr_ptes BUG
> +	 */
> +	p = mmap64(q + hpage_size, hpage_size, PROT_READ|PROT_WRITE,
> +		   MAP_FIXED|MAP_PRIVATE|MAP_ANON, -1, 0);
> +	if (p != q + hpage_size)
> +		tst_brk(TBROK, "mmap() offset 4GB: %s", strerror(errno));

Here as well.

> +	pi = p;
> +	/* Touch one page to allocate its page table */
> +	*pi = 0;
> +#endif
> +
> +	/* Replace top hpage by hpage mapping at confusing file offset */
> +	p = mmap64(q + 2*hpage_size, hpage_size, PROT_READ|PROT_WRITE,
> +		 MAP_FIXED|MAP_PRIVATE, fd, buggy_offset);
> +	if (p != q + 2*hpage_size)
> +		tst_brk(TBROK, "mmap() buggy offset 0x%llx", buggy_offset);

Here as well.

> +	pi = p;
> +	/* Touch the low page with something non-zero */
> +	*pi = 1;
> +
> +	err = ftruncate64(fd, truncate_point);
> +	if (err) {
> +		tst_res(TFAIL, "ftruncate failed");
> +		goto cleanup;
> +	}

And here as well.

> +	SAFE_SIGACTION(SIGBUS, &sa_fail, NULL);
> +	if (sigsetjmp(sig_escape, 1) == 0)
> +		if (*pi != 1) {
> +			tst_res(TFAIL, "Data 1 has changed!");
> +			goto cleanup;
> +		}
> +
> +	SAFE_SIGACTION(SIGBUS, &sa_pass, NULL);
> +	if (sigsetjmp(sig_escape, 1) == 0)
> +		*qi;
> +	else
> +		sigbus_count++;
> +	if (sigbus_count != 1)
> +		/* Should have SIGBUSed above */
> +		tst_res(TFAIL, "Didn't SIGBUS on truncated page.");
> +	if (test_pass == 1)
> +		tst_res(TPASS, "Expected SIGBUS");

Why do we have both test_pass and sigbus count here?

Shouldn't be a single flag set from the signal handler enough?

> +cleanup:
> +	SAFE_MUNMAP(q, 3*hpage_size);
> +	SAFE_MUNMAP(p, hpage_size);
> +}
> +
> +static void setup(void)
> +{
> +	long long truncate_point;
> +
> +	page_size = getpagesize();
> +	hpage_size = tst_get_hugepage_size();
> +	fd = tst_creat_unlinked(MNTPOINT, 0, 0600);
> +	truncate_point = FOURGIG;
> +	if (hpage_size > truncate_point)
> +		tst_brk(TCONF, "Huge page size is too large!");
> +	if (truncate_point % hpage_size > 0)
> +		tst_brk(TCONF, "Truncation point is not aligned to huge page size!");

Hmm, I find this unlinkely to ever happen. As far as I can tell all
currently supported hugepage sizes are multiples of two and as such they
by definition divide 4GB without reminder.

> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd >= 0)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.tags = (struct tst_tag[]) {
> +		{"linux-git", "856fc2950555"},
> +		{}
> +	},
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.needs_hugetlbfs = 1,
> +	.hugepages = {4, TST_NEEDS},
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run_test,
> +};
> -- 
> 2.43.5
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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