[LTP] [PATCH] syscalls/madvise11: Ignore unpoison failure under kernel lockdown

[LTP] [PATCH] syscalls/madvise11: Ignore unpoison failure under kernel lockdown

[Martin Doucha]

When newer kernels are under lockdown, the unpoison-pfn sysfile
still appears writable to root but open() will always return EPERM.
This causes madvise11 to fail with TWARN during cleanup when run
with SecureBoot enabled.

Ignore the open(unpoison-pfn) failure due to lockdown and exit
successfully without cleanup. The test should not be skipped because
the leftover soft-offlined pages can trigger failures in later tests
and indirectly expose kernel bugs in hwpoison.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 2cb45d00a..e63e28f8a 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -323,7 +323,20 @@ static int open_unpoison_pfn(void)
 	if (!mnt)
 		return -1;
 
-	return SAFE_OPEN(debugfs_fp, O_WRONLY);
+	TEST(open(debugfs_fp, O_WRONLY));
+
+	if (TST_RET == -1 && TST_ERR == EPERM && tst_lockdown_enabled()) {
+		tst_res(TINFO,
+			"Cannot restore soft-offlined memory due to lockdown");
+		return TST_RET;
+	}
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "open(%s) failed", debugfs_fp);
+	else if (TST_RET < 0)
+		tst_brk(TBROK | TTERRNO, "Invalid open() return value");
+
+	return TST_RET;
 }
 
 /*
-- 
2.40.0


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

Re: [LTP] [PATCH] syscalls/madvise11: Ignore unpoison failure under kernel lockdown

[Petr Vorel]

Hi Martin,

...
> -	return SAFE_OPEN(debugfs_fp, O_WRONLY);
> +	TEST(open(debugfs_fp, O_WRONLY));
> +
> +	if (TST_RET == -1 && TST_ERR == EPERM && tst_lockdown_enabled()) {
> +		tst_res(TINFO,
> +			"Cannot restore soft-offlined memory due to lockdown");
> +		return TST_RET;
> +	}
> +
> +	if (TST_RET == -1)
> +		tst_brk(TBROK | TTERRNO, "open(%s) failed", debugfs_fp);
> +	else if (TST_RET < 0)
> +		tst_brk(TBROK | TTERRNO, "Invalid open() return value");
nit: I'd print the return value as we do in safe_open():
		tst_brk(TBROK | TTERRNO, "Invalid open() return value %d", TST_RET);

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for fixing this!

Kind regards,
Petr

> +
> +	return TST_RET;
>  }


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

Re: [LTP] [PATCH] syscalls/madvise11: Ignore unpoison failure under kernel lockdown

[Martin Doucha]

On 20. 06. 23 10:24, Petr Vorel wrote:
> Hi Martin,
> 
> ...
>> -	return SAFE_OPEN(debugfs_fp, O_WRONLY);
>> +	TEST(open(debugfs_fp, O_WRONLY));
>> +
>> +	if (TST_RET == -1 && TST_ERR == EPERM && tst_lockdown_enabled()) {
>> +		tst_res(TINFO,
>> +			"Cannot restore soft-offlined memory due to lockdown");
>> +		return TST_RET;
>> +	}
>> +
>> +	if (TST_RET == -1)
>> +		tst_brk(TBROK | TTERRNO, "open(%s) failed", debugfs_fp);
>> +	else if (TST_RET < 0)
>> +		tst_brk(TBROK | TTERRNO, "Invalid open() return value");
> nit: I'd print the return value as we do in safe_open():
> 		tst_brk(TBROK | TTERRNO, "Invalid open() return value %d", TST_RET);

Good point. Though TST_RET is long int so it should be %ld. Should I 
send a v2?

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

Re: [LTP] [PATCH] syscalls/madvise11: Ignore unpoison failure under kernel lockdown

[Petr Vorel]

> On 20. 06. 23 10:24, Petr Vorel wrote:
> > Hi Martin,

> > ...
> > > -	return SAFE_OPEN(debugfs_fp, O_WRONLY);
> > > +	TEST(open(debugfs_fp, O_WRONLY));
> > > +
> > > +	if (TST_RET == -1 && TST_ERR == EPERM && tst_lockdown_enabled()) {
> > > +		tst_res(TINFO,
> > > +			"Cannot restore soft-offlined memory due to lockdown");
> > > +		return TST_RET;
> > > +	}
> > > +
> > > +	if (TST_RET == -1)
> > > +		tst_brk(TBROK | TTERRNO, "open(%s) failed", debugfs_fp);
> > > +	else if (TST_RET < 0)
> > > +		tst_brk(TBROK | TTERRNO, "Invalid open() return value");
> > nit: I'd print the return value as we do in safe_open():
> > 		tst_brk(TBROK | TTERRNO, "Invalid open() return value %d", TST_RET);

> Good point. Though TST_RET is long int so it should be %ld. Should I send a
> v2?

Yes, please.

Kind regards,
Petr

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