[LTP] [PATCH 0/4] Add .skip_in_secureboot flag

[LTP] [PATCH 0/4] Add .skip_in_secureboot flag

[Petr Vorel]

Petr Vorel (4):
  tst_lockdown: Check other lockdown configuration
  lib: Add .skip_in_secureboot flag
  {delete,finit,init}_module0[1-3]: Skip on SecureBoot
  doc/c-api: Document .skip_in_* flags

 doc/c-test-api.txt                              |  6 ++++++
 doc/test-writing-guidelines.txt                 |  1 +
 include/tst_test.h                              |  4 ++++
 lib/tst_lockdown.c                              | 11 +++++++----
 lib/tst_test.c                                  |  3 +++
 .../syscalls/delete_module/delete_module01.c    |  2 ++
 .../syscalls/delete_module/delete_module03.c    |  2 ++
 .../syscalls/finit_module/finit_module01.c      |  2 ++
 .../syscalls/finit_module/finit_module02.c      | 17 +++++++++++++----
 .../kernel/syscalls/init_module/init_module01.c |  2 ++
 .../kernel/syscalls/init_module/init_module02.c | 16 ++++++++++++----
 11 files changed, 54 insertions(+), 12 deletions(-)

-- 
2.40.1


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

[LTP] [PATCH 1/4] tst_lockdown: Check other lockdown configuration

[Petr Vorel]

Originally we checked only CONFIG_EFI_SECURE_BOOT_LOCK_DOWN=y
(non-mainline patch from 2017 [1]. Various distros (older releases) use
other newer non-mainline patch [2] (originally from Fedora 32), which with
CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT=y forces lockdown, when in secure boot.

[1] https://lore.kernel.org/lkml/149141204578.30815.1929675368430800975.stgit@warthog.procyon.org.uk/
[2] https://lore.kernel.org/lkml/150842483945.7923.12778302394414653081.stgit@warthog.procyon.org.uk/

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_lockdown.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
index 26a57b6a1..f91bc919d 100644
--- a/lib/tst_lockdown.c
+++ b/lib/tst_lockdown.c
@@ -47,18 +47,21 @@ int tst_lockdown_enabled(void)
 {
 	char line[BUFSIZ];
 	FILE *file;
+	char flag;
 
 	if (access(PATH_LOCKDOWN, F_OK) != 0) {
-		char flag;
-
+		/* SecureBoot enabled could mean integrity lockdown (non-mainline version) */
 		flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN");
-
-		/* SecureBoot enabled could mean integrity lockdown */
 		if (flag == 'y' && tst_secureboot_enabled() > 0)
 			return 1;
 
 		tst_res(TINFO, "Unable to determine system lockdown state");
 		return 0;
+	} else {
+		/* SecureBoot forces lockdown (non-mainline version) */
+		flag = tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT");
+		if (flag == 'y' && tst_secureboot_enabled() > 0)
+			return 1;
 	}
 
 	file = SAFE_FOPEN(PATH_LOCKDOWN, "r");
-- 
2.40.1


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

[LTP] [PATCH 2/4] lib: Add .skip_in_secureboot flag

[Petr Vorel]

This will be used in module related tests.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/test-writing-guidelines.txt | 1 +
 include/tst_test.h              | 4 ++++
 lib/tst_test.c                  | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index b83a6fdb6..6d1a69165 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -393,6 +393,7 @@ https://github.com/linux-test-project/ltp/wiki/Shell-Test-API[Shell Test API].
 | '.skip_filesystems' | 'TST_SKIP_FILESYSTEMS'
 | '.skip_in_compat' | –
 | '.skip_in_lockdown' | –
+| '.skip_in_secureboot' | –
 | '.supported_archs' | not applicable
 | '.tags' | –
 | '.taint_check' | –
diff --git a/include/tst_test.h b/include/tst_test.h
index 22acfba59..0ac492a80 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -177,6 +177,7 @@ struct tst_test {
 	int child_needs_reinit:1;
 	int needs_devfs:1;
 	int restore_wallclock:1;
+
 	/*
 	 * If set the test function will be executed for all available
 	 * filesystems and the current filesystem type would be set in the
@@ -186,8 +187,11 @@ struct tst_test {
 	 * to the test function.
 	 */
 	int all_filesystems:1;
+
 	int skip_in_lockdown:1;
+	int skip_in_secureboot:1;
 	int skip_in_compat:1;
+
 	/*
 	 * If set, the hugetlbfs will be mounted at .mntpoint.
 	 */
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 04da456c6..8f7223b0e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1160,6 +1160,9 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
 		tst_brk(TCONF, "Kernel is locked down, skipping test");
 
+	if (tst_test->skip_in_secureboot && tst_secureboot_enabled())
+		tst_brk(TCONF, "SecureBoot enabled, skipping test");
+
 	if (tst_test->skip_in_compat && TST_ABI != tst_kernel_bits())
 		tst_brk(TCONF, "Not supported in 32-bit compat mode");
 
-- 
2.40.1


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

[LTP] [PATCH 3/4] {delete, finit, init}_module0[1-3]: Skip on SecureBoot

[Petr Vorel]

Enabled SecureBoot requires signed modules (regardless lockdown state).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../syscalls/delete_module/delete_module01.c    |  2 ++
 .../syscalls/delete_module/delete_module03.c    |  2 ++
 .../syscalls/finit_module/finit_module01.c      |  2 ++
 .../syscalls/finit_module/finit_module02.c      | 17 +++++++++++++----
 .../kernel/syscalls/init_module/init_module01.c |  2 ++
 .../kernel/syscalls/init_module/init_module02.c | 16 ++++++++++++----
 6 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
index 6ecd2cad1..08597cfd6 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module01.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
@@ -52,6 +52,8 @@ static struct tst_test test = {
 	.needs_root = 1,
 	/* lockdown requires signed modules */
 	.skip_in_lockdown = 1,
+	/* SecureBoot requires signed modules */
+	.skip_in_secureboot = 1,
 	.cleanup = cleanup,
 	.test_all = do_delete_module,
 };
diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
index 863d36188..a4b5108f0 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module03.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
@@ -74,6 +74,8 @@ static struct tst_test test = {
 	.needs_root = 1,
 	/* lockdown requires signed modules */
 	.skip_in_lockdown = 1,
+	/* SecureBoot requires signed modules */
+	.skip_in_secureboot = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = do_delete_module,
diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
index f960b2e40..660b567f5 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module01.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
@@ -51,4 +51,6 @@ static struct tst_test test = {
 	.needs_root = 1,
 	/* lockdown requires signed modules */
 	.skip_in_lockdown = 1,
+	/* SecureBoot requires signed modules */
+	.skip_in_secureboot = 1,
 };
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index a7434de7d..4f5962829 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -25,7 +25,7 @@
 static char *mod_path;
 
 static int fd, fd_zero, fd_invalid = -1, fd_dir;
-static int kernel_lockdown;
+static int kernel_lockdown, secure_boot;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
 static struct tst_cap cap_drop = TST_CAP(TST_CAP_DROP, CAP_SYS_MODULE);
@@ -84,6 +84,8 @@ static void setup(void)
 	tst_module_exists(MODULE_NAME, &mod_path);
 
 	kernel_lockdown = tst_lockdown_enabled();
+	secure_boot = tst_secureboot_enabled();
+
 	SAFE_MKDIR(TEST_DIR, 0700);
 	fd_dir = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
 
@@ -102,9 +104,16 @@ static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
-	if (tc->skip_in_lockdown && kernel_lockdown) {
-		tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
-		return;
+	if (tc->skip_in_lockdown) {
+		if (secure_boot) {
+			tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name);
+			return;
+		}
+
+		if (kernel_lockdown) {
+			tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
+			return;
+		}
 	}
 
 	fd = SAFE_OPEN(mod_path, tc->open_flags);
diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
index 79e567cd6..80b2b77cc 100644
--- a/testcases/kernel/syscalls/init_module/init_module01.c
+++ b/testcases/kernel/syscalls/init_module/init_module01.c
@@ -55,4 +55,6 @@ static struct tst_test test = {
 	.needs_root = 1,
 	/* lockdown requires signed modules */
 	.skip_in_lockdown = 1,
+	/* SecureBoot requires signed modules */
+	.skip_in_secureboot = 1,
 };
diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
index ad6569a06..4acbfbcd1 100644
--- a/testcases/kernel/syscalls/init_module/init_module02.c
+++ b/testcases/kernel/syscalls/init_module/init_module02.c
@@ -22,7 +22,7 @@
 #define MODULE_NAME	"init_module.ko"
 
 static unsigned long size, zero_size;
-static int kernel_lockdown;
+static int kernel_lockdown, secure_boot;
 static void *buf, *faulty_buf, *null_buf;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
@@ -54,6 +54,7 @@ static void setup(void)
 	tst_module_exists(MODULE_NAME, NULL);
 
 	kernel_lockdown = tst_lockdown_enabled();
+	secure_boot = tst_secureboot_enabled();
 	fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC);
 	SAFE_FSTAT(fd, &sb);
 	size = sb.st_size;
@@ -67,9 +68,16 @@ static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
-	if (tc->skip_in_lockdown && kernel_lockdown) {
-		tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
-		return;
+	if (tc->skip_in_lockdown) {
+		if (secure_boot) {
+			tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name);
+			return;
+		}
+
+		if (kernel_lockdown) {
+			tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
+			return;
+		}
 	}
 
 	if (tc->cap)
-- 
2.40.1


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

[LTP] [PATCH 4/4] doc/c-api: Document .skip_in_* flags

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/c-test-api.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 07c069ced..74871e6c8 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -2412,6 +2412,12 @@ static struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
+1.41 Skipping test based on system state
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Test can be skipped on various conditions: on enabled SecureBoot
+('.skip_in_secureboot = 1'), lockdown ('.skip_in_lockdown = 1') or in 32-bit
+compat mode ('.skip_in_compat = 1').
+
 2. Common problems
 ------------------
 
-- 
2.40.1


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

Re: [LTP] [PATCH 1/4] tst_lockdown: Check other lockdown configuration

[Martin Doucha]

Hi,

On 20. 07. 23 17:02, Petr Vorel wrote:
> Originally we checked only CONFIG_EFI_SECURE_BOOT_LOCK_DOWN=y
> (non-mainline patch from 2017 [1]. Various distros (older releases) use
> other newer non-mainline patch [2] (originally from Fedora 32), which with
> CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT=y forces lockdown, when in secure boot.
> 
> [1] https://lore.kernel.org/lkml/149141204578.30815.1929675368430800975.stgit@warthog.procyon.org.uk/
> [2] https://lore.kernel.org/lkml/150842483945.7923.12778302394414653081.stgit@warthog.procyon.org.uk/
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   lib/tst_lockdown.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> index 26a57b6a1..f91bc919d 100644
> --- a/lib/tst_lockdown.c
> +++ b/lib/tst_lockdown.c
> @@ -47,18 +47,21 @@ int tst_lockdown_enabled(void)
>   {
>   	char line[BUFSIZ];
>   	FILE *file;
> +	char flag;
>   
>   	if (access(PATH_LOCKDOWN, F_OK) != 0) {
> -		char flag;
> -
> +		/* SecureBoot enabled could mean integrity lockdown (non-mainline version) */
>   		flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN");
> -
> -		/* SecureBoot enabled could mean integrity lockdown */
>   		if (flag == 'y' && tst_secureboot_enabled() > 0)
>   			return 1;
>   
>   		tst_res(TINFO, "Unable to determine system lockdown state");
>   		return 0;
> +	} else {

There should be no "else" branch here. The code above should look like this:

int flag;
flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN") == 'y';
flag |= tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT") == 'y';
if (flag && tst_secureboot_enabled() > 0)
	return 1;

> +		/* SecureBoot forces lockdown (non-mainline version) */
> +		flag = tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT");
> +		if (flag == 'y' && tst_secureboot_enabled() > 0)
> +			return 1;
>   	}
>   
>   	file = SAFE_FOPEN(PATH_LOCKDOWN, "r");

-- 
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 2/4] lib: Add .skip_in_secureboot flag

[Martin Doucha]

Hi,
Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 20. 07. 23 17:02, Petr Vorel wrote:
> This will be used in module related tests.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   doc/test-writing-guidelines.txt | 1 +
>   include/tst_test.h              | 4 ++++
>   lib/tst_test.c                  | 3 +++
>   3 files changed, 8 insertions(+)
> 
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index b83a6fdb6..6d1a69165 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -393,6 +393,7 @@ https://github.com/linux-test-project/ltp/wiki/Shell-Test-API[Shell Test API].
>   | '.skip_filesystems' | 'TST_SKIP_FILESYSTEMS'
>   | '.skip_in_compat' | –
>   | '.skip_in_lockdown' | –
> +| '.skip_in_secureboot' | –
>   | '.supported_archs' | not applicable
>   | '.tags' | –
>   | '.taint_check' | –
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 22acfba59..0ac492a80 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -177,6 +177,7 @@ struct tst_test {
>   	int child_needs_reinit:1;
>   	int needs_devfs:1;
>   	int restore_wallclock:1;
> +
>   	/*
>   	 * If set the test function will be executed for all available
>   	 * filesystems and the current filesystem type would be set in the
> @@ -186,8 +187,11 @@ struct tst_test {
>   	 * to the test function.
>   	 */
>   	int all_filesystems:1;
> +
>   	int skip_in_lockdown:1;
> +	int skip_in_secureboot:1;
>   	int skip_in_compat:1;
> +
>   	/*
>   	 * If set, the hugetlbfs will be mounted at .mntpoint.
>   	 */
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 04da456c6..8f7223b0e 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1160,6 +1160,9 @@ static void do_setup(int argc, char *argv[])
>   	if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
>   		tst_brk(TCONF, "Kernel is locked down, skipping test");
>   
> +	if (tst_test->skip_in_secureboot && tst_secureboot_enabled())
> +		tst_brk(TCONF, "SecureBoot enabled, skipping test");
> +
>   	if (tst_test->skip_in_compat && TST_ABI != tst_kernel_bits())
>   		tst_brk(TCONF, "Not supported in 32-bit compat mode");
>   

-- 
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 3/4] {delete, finit, init}_module0[1-3]: Skip on SecureBoot

[Martin Doucha]

Hi,

On 20. 07. 23 17:02, Petr Vorel wrote:
> Enabled SecureBoot requires signed modules (regardless lockdown state).
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   .../syscalls/delete_module/delete_module01.c    |  2 ++
>   .../syscalls/delete_module/delete_module03.c    |  2 ++
>   .../syscalls/finit_module/finit_module01.c      |  2 ++
>   .../syscalls/finit_module/finit_module02.c      | 17 +++++++++++++----
>   .../kernel/syscalls/init_module/init_module01.c |  2 ++
>   .../kernel/syscalls/init_module/init_module02.c | 16 ++++++++++++----
>   6 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
> index 6ecd2cad1..08597cfd6 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> @@ -52,6 +52,8 @@ static struct tst_test test = {
>   	.needs_root = 1,
>   	/* lockdown requires signed modules */
>   	.skip_in_lockdown = 1,
> +	/* SecureBoot requires signed modules */

Nit: the two comments could be merged into one.

> +	.skip_in_secureboot = 1,
>   	.cleanup = cleanup,
>   	.test_all = do_delete_module,
>   };
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
> index 863d36188..a4b5108f0 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module03.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
> @@ -74,6 +74,8 @@ static struct tst_test test = {
>   	.needs_root = 1,
>   	/* lockdown requires signed modules */
>   	.skip_in_lockdown = 1,
> +	/* SecureBoot requires signed modules */
> +	.skip_in_secureboot = 1,
>   	.setup = setup,
>   	.cleanup = cleanup,
>   	.test_all = do_delete_module,
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
> index f960b2e40..660b567f5 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module01.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
> @@ -51,4 +51,6 @@ static struct tst_test test = {
>   	.needs_root = 1,
>   	/* lockdown requires signed modules */
>   	.skip_in_lockdown = 1,
> +	/* SecureBoot requires signed modules */
> +	.skip_in_secureboot = 1,
>   };
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> index a7434de7d..4f5962829 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> @@ -25,7 +25,7 @@
>   static char *mod_path;
>   
>   static int fd, fd_zero, fd_invalid = -1, fd_dir;
> -static int kernel_lockdown;
> +static int kernel_lockdown, secure_boot;
>   
>   static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
>   static struct tst_cap cap_drop = TST_CAP(TST_CAP_DROP, CAP_SYS_MODULE);
> @@ -84,6 +84,8 @@ static void setup(void)
>   	tst_module_exists(MODULE_NAME, &mod_path);
>   
>   	kernel_lockdown = tst_lockdown_enabled();
> +	secure_boot = tst_secureboot_enabled();
> +
>   	SAFE_MKDIR(TEST_DIR, 0700);
>   	fd_dir = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
>   
> @@ -102,9 +104,16 @@ static void run(unsigned int n)
>   {
>   	struct tcase *tc = &tcases[n];
>   
> -	if (tc->skip_in_lockdown && kernel_lockdown) {
> -		tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> -		return;
> +	if (tc->skip_in_lockdown) {
> +		if (secure_boot) {
> +			tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name);
> +			return;
> +		}
> +
> +		if (kernel_lockdown) {
> +			tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> +			return;
> +		}

It'd be better to just change the original TCONF message to something 
like "Cannot load unsigned modules, skipping %s". Adding a TINFO message 
"Lockdown on/off" to tst_lockdown_enabled() would provide the 
explanation why.

>   	}
>   
>   	fd = SAFE_OPEN(mod_path, tc->open_flags);
> diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
> index 79e567cd6..80b2b77cc 100644
> --- a/testcases/kernel/syscalls/init_module/init_module01.c
> +++ b/testcases/kernel/syscalls/init_module/init_module01.c
> @@ -55,4 +55,6 @@ static struct tst_test test = {
>   	.needs_root = 1,
>   	/* lockdown requires signed modules */
>   	.skip_in_lockdown = 1,
> +	/* SecureBoot requires signed modules */
> +	.skip_in_secureboot = 1,
>   };
> diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
> index ad6569a06..4acbfbcd1 100644
> --- a/testcases/kernel/syscalls/init_module/init_module02.c
> +++ b/testcases/kernel/syscalls/init_module/init_module02.c
> @@ -22,7 +22,7 @@
>   #define MODULE_NAME	"init_module.ko"
>   
>   static unsigned long size, zero_size;
> -static int kernel_lockdown;
> +static int kernel_lockdown, secure_boot;
>   static void *buf, *faulty_buf, *null_buf;
>   
>   static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
> @@ -54,6 +54,7 @@ static void setup(void)
>   	tst_module_exists(MODULE_NAME, NULL);
>   
>   	kernel_lockdown = tst_lockdown_enabled();
> +	secure_boot = tst_secureboot_enabled();
>   	fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC);
>   	SAFE_FSTAT(fd, &sb);
>   	size = sb.st_size;
> @@ -67,9 +68,16 @@ static void run(unsigned int n)
>   {
>   	struct tcase *tc = &tcases[n];
>   
> -	if (tc->skip_in_lockdown && kernel_lockdown) {
> -		tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> -		return;
> +	if (tc->skip_in_lockdown) {
> +		if (secure_boot) {
> +			tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name);
> +			return;
> +		}
> +
> +		if (kernel_lockdown) {
> +			tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> +			return;
> +		}

Same here.

>   	}
>   
>   	if (tc->cap)

-- 
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 4/4] doc/c-api: Document .skip_in_* flags

[Martin Doucha]

Hi,
Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 20. 07. 23 17:02, Petr Vorel wrote:
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   doc/c-test-api.txt | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
> index 07c069ced..74871e6c8 100644
> --- a/doc/c-test-api.txt
> +++ b/doc/c-test-api.txt
> @@ -2412,6 +2412,12 @@ static struct tst_test test = {
>   };
>   -------------------------------------------------------------------------------
>   
> +1.41 Skipping test based on system state
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Test can be skipped on various conditions: on enabled SecureBoot
> +('.skip_in_secureboot = 1'), lockdown ('.skip_in_lockdown = 1') or in 32-bit
> +compat mode ('.skip_in_compat = 1').
> +
>   2. Common problems
>   ------------------
>   

-- 
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 3/4] {delete, finit, init}_module0[1-3]: Skip on SecureBoot

[Petr Vorel]

Hi Martin,

> > +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> > @@ -52,6 +52,8 @@ static struct tst_test test = {
> >   	.needs_root = 1,
> >   	/* lockdown requires signed modules */
> >   	.skip_in_lockdown = 1,
> > +	/* SecureBoot requires signed modules */

> Nit: the two comments could be merged into one.
+1

...
> > @@ -102,9 +104,16 @@ static void run(unsigned int n)
> >   {
> >   	struct tcase *tc = &tcases[n];
> > -	if (tc->skip_in_lockdown && kernel_lockdown) {
> > -		tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> > -		return;
> > +	if (tc->skip_in_lockdown) {
> > +		if (secure_boot) {
> > +			tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name);
> > +			return;
> > +		}
> > +
> > +		if (kernel_lockdown) {
> > +			tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name);
> > +			return;
> > +		}

> It'd be better to just change the original TCONF message to something like
> "Cannot load unsigned modules, skipping %s". Adding a TINFO message
> "Lockdown on/off" to tst_lockdown_enabled() would provide the explanation
> why.

+1

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] tst_lockdown: Check other lockdown configuration

[Petr Vorel]

Hi Martin,

...
> > +	char flag;
> >   	if (access(PATH_LOCKDOWN, F_OK) != 0) {
> > -		char flag;
> > -
> > +		/* SecureBoot enabled could mean integrity lockdown (non-mainline version) */
> >   		flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN");
> > -
> > -		/* SecureBoot enabled could mean integrity lockdown */
> >   		if (flag == 'y' && tst_secureboot_enabled() > 0)
> >   			return 1;
> >   		tst_res(TINFO, "Unable to determine system lockdown state");
> >   		return 0;
> > +	} else {

> There should be no "else" branch here. The code above should look like this:

> int flag;
> flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN") == 'y';
> flag |= tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT") == 'y';
> if (flag && tst_secureboot_enabled() > 0)
> 	return 1;

Good point. I don't know why I thought that the other not-yet upstreamed patch
created lockdown file. I'll send v2.

Kind regards,
Petr

> > +		/* SecureBoot forces lockdown (non-mainline version) */
> > +		flag = tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT");
> > +		if (flag == 'y' && tst_secureboot_enabled() > 0)
> > +			return 1;
> >   	}
> >   	file = SAFE_FOPEN(PATH_LOCKDOWN, "r");

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