[LTP] [PATCH 1/3] Add skip_in_lockdown flag to struct tst_test

[LTP] [PATCH 1/3] Add skip_in_lockdown flag to struct tst_test

[Martin Doucha]

When the flag is set to 1, the LTP library will call tst_lockdown_enabled()
during initialization and exit with TCONF if kernel lockdown is detected.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/tst_test.h | 1 +
 lib/tst_test.c     | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/tst_test.h b/include/tst_test.h
index 6ad355506..c7d77eb09 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -157,6 +157,7 @@ struct tst_test {
 	 * to the test function.
 	 */
 	int all_filesystems:1;
+	int skip_in_lockdown:1;
 
 	/*
 	 * The skip_filesystem is a NULL terminated list of filesystems the
diff --git a/lib/tst_test.c b/lib/tst_test.c
index f4d9f8e3b..c7c77596c 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -957,6 +957,9 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->min_kver)
 		check_kver();
 
+	if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
+		tst_brk(TCONF, "Kernel is locked down, skipping test");
+
 	if (tst_test->needs_cmds) {
 		const char *cmd;
 		char path[PATH_MAX];
-- 
2.32.0

[LTP] [PATCH 2/3] Replace existing lockdown checks with skip_in_lockdown

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../kernel/syscalls/delete_module/delete_module01.c    |  6 ++----
 .../kernel/syscalls/delete_module/delete_module03.c    |  6 ++----
 testcases/kernel/syscalls/ioperm/ioperm01.c            |  6 ++----
 testcases/kernel/syscalls/ioperm/ioperm02.c            |  6 ++----
 testcases/kernel/syscalls/iopl/iopl01.c                | 10 ++--------
 testcases/kernel/syscalls/iopl/iopl02.c                |  6 ++----
 6 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
index cec20c19c..c4f6978c1 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module01.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
@@ -25,10 +25,6 @@ static int module_loaded;
 
 static void do_delete_module(void)
 {
-	/* lockdown requires signed modules */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-
 	if (module_loaded == 0) {
 		tst_module_load(MODULE_NAME_KO, NULL);
 		module_loaded = 1;
@@ -53,6 +49,8 @@ static void cleanup(void)
 
 static struct tst_test test = {
 	.needs_root = 1,
+	/* lockdown requires signed modules */
+	.skip_in_lockdown = 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 56cef459f..863d36188 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module03.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
@@ -50,10 +50,6 @@ static void do_delete_module(void)
 
 static void setup(void)
 {
-	/* lockdown requires signed modules */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-
 	/* Load first kernel module */
 	tst_module_load(DUMMY_MOD_KO, NULL);
 	dummy_mod_loaded = 1;
@@ -76,6 +72,8 @@ static void cleanup(void)
 
 static struct tst_test test = {
 	.needs_root = 1,
+	/* lockdown requires signed modules */
+	.skip_in_lockdown = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = do_delete_module,
diff --git a/testcases/kernel/syscalls/ioperm/ioperm01.c b/testcases/kernel/syscalls/ioperm/ioperm01.c
index 6c50a0b75..8f2cc684d 100644
--- a/testcases/kernel/syscalls/ioperm/ioperm01.c
+++ b/testcases/kernel/syscalls/ioperm/ioperm01.c
@@ -42,10 +42,6 @@ static void verify_ioperm(void)
 
 static void setup(void)
 {
-	/* ioperm() is restricted under kernel lockdown. */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-
 	/*
 	 * The value of IO_BITMAP_BITS (include/asm-i386/processor.h) changed
 	 * from kernel 2.6.8 to permit 16-bits ioperm
@@ -70,6 +66,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.test_all = verify_ioperm,
 	.needs_root = 1,
+	/* ioperm() is restricted under kernel lockdown. */
+	.skip_in_lockdown = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 };
diff --git a/testcases/kernel/syscalls/ioperm/ioperm02.c b/testcases/kernel/syscalls/ioperm/ioperm02.c
index 80dcb992e..33c501904 100644
--- a/testcases/kernel/syscalls/ioperm/ioperm02.c
+++ b/testcases/kernel/syscalls/ioperm/ioperm02.c
@@ -45,10 +45,6 @@ static struct tcase_t {
 
 static void setup(void)
 {
-	/* ioperm() is restricted under kernel lockdown. */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-
 	/*
 	 * The value of IO_BITMAP_BITS (include/asm-i386/processor.h) changed
 	 * from kernel 2.6.8 to permit 16-bits (65536) ioperm
@@ -92,6 +88,8 @@ static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = verify_ioperm,
 	.needs_root = 1,
+	/* ioperm() is restricted under kernel lockdown. */
+	.skip_in_lockdown = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 };
diff --git a/testcases/kernel/syscalls/iopl/iopl01.c b/testcases/kernel/syscalls/iopl/iopl01.c
index dcf2cc406..70e2a7ff1 100644
--- a/testcases/kernel/syscalls/iopl/iopl01.c
+++ b/testcases/kernel/syscalls/iopl/iopl01.c
@@ -42,13 +42,6 @@ static void verify_iopl(void)
 	}
 }
 
-static void setup(void)
-{
-	/* iopl() is restricted under kernel lockdown. */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-}
-
 static void cleanup(void)
 {
 	/*
@@ -61,7 +54,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.test_all = verify_iopl,
 	.needs_root = 1,
-	.setup = setup,
+	/* iopl() is restricted under kernel lockdown. */
+	.skip_in_lockdown = 1,
 	.cleanup = cleanup,
 };
 
diff --git a/testcases/kernel/syscalls/iopl/iopl02.c b/testcases/kernel/syscalls/iopl/iopl02.c
index a6135ddf3..7301442f9 100644
--- a/testcases/kernel/syscalls/iopl/iopl02.c
+++ b/testcases/kernel/syscalls/iopl/iopl02.c
@@ -53,10 +53,6 @@ static void setup(void)
 {
 	struct passwd *pw;
 
-	/* iopl() is restricted under kernel lockdown. */
-	if (tst_lockdown_enabled())
-		tst_brk(TCONF, "Kernel is locked down, skip this test");
-
 	pw = SAFE_GETPWNAM("nobody");
 	SAFE_SETEUID(pw->pw_uid);
 }
@@ -70,6 +66,8 @@ static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = verify_iopl,
 	.needs_root = 1,
+	/* iopl() is restricted under kernel lockdown. */
+	.skip_in_lockdown = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 };
-- 
2.32.0

[LTP] [PATCH 2/3] Replace existing lockdown checks with skip_in_lockdown

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] Add lockdown checks to init_module* and finit_module* tests

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../syscalls/finit_module/finit_module01.c    |  2 ++
 .../syscalls/finit_module/finit_module02.c    | 23 +++++++++++++--
 .../syscalls/init_module/init_module01.c      |  2 ++
 .../syscalls/init_module/init_module02.c      | 29 +++++++++++++------
 4 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
index 9c34282e1..21c35f101 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 = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	/* lockdown requires signed modules */
+	.skip_in_lockdown = 1,
 };
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index 9d9255c6d..503c8e994 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -25,6 +25,7 @@
 static char *mod_path;
 
 static int fd, fd_zero, fd_invalid = -1, fd_dir;
+static int kernel_lockdown;
 
 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);
@@ -64,11 +65,19 @@ static void dir_setup(struct tcase *tc)
 		tc->exp_errno = EINVAL;
 }
 
+static void lockdown_setup(struct tcase *tc)
+{
+	if (kernel_lockdown)
+		tc->exp_errno = EPERM;
+}
+
 static struct tcase tcases[] = {
 	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, bad_fd_setup},
 	{"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
-	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT, NULL},
-	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
+	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT,
+		lockdown_setup},
+	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0,
+		EINVAL, lockdown_setup},
 	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, NULL},
 	{"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, NULL},
 	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, NULL},
@@ -84,6 +93,7 @@ static void setup(void)
 
 	tst_module_exists(MODULE_NAME, &mod_path);
 
+	kernel_lockdown = tst_lockdown_enabled();
 	SAFE_MKDIR(TEST_DIR, 0700);
 	fd_dir = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
 
@@ -108,8 +118,15 @@ static void run(unsigned int n)
 		tst_cap_action(&cap_drop);
 
 	/* Insert module twice */
-	if (tc->exp_errno == EEXIST)
+	if (tc->exp_errno == EEXIST) {
+		if (kernel_lockdown) {
+			tst_res(TCONF, "Kernel is locked down, skipping %s",
+				tc->name);
+			return;
+		}
+
 		tst_module_load(MODULE_NAME, NULL);
+	}
 
 	TST_EXP_FAIL(finit_module(*tc->fd, tc->param, tc->flags), tc->exp_errno,
 		     "TestName: %s", tc->name);
diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
index 2f47eed32..79e567cd6 100644
--- a/testcases/kernel/syscalls/init_module/init_module01.c
+++ b/testcases/kernel/syscalls/init_module/init_module01.c
@@ -53,4 +53,6 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	/* lockdown requires signed modules */
+	.skip_in_lockdown = 1,
 };
diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
index 3953f4f61..dac99a4da 100644
--- a/testcases/kernel/syscalls/init_module/init_module02.c
+++ b/testcases/kernel/syscalls/init_module/init_module02.c
@@ -22,6 +22,7 @@
 #define MODULE_NAME	"init_module.ko"
 
 static unsigned long size, zero_size;
+static int kernel_lockdown;
 static void *buf, *faulty_buf, *null_buf;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
@@ -34,14 +35,15 @@ static struct tcase {
 	const char *param;
 	int cap;
 	int exp_errno;
+	int lockdown_errno;
 } tcases[] = {
-	{"NULL-buffer", &null_buf, &size, "", 0, EFAULT},
-	{"faulty-buffer", &faulty_buf, &size, "", 0, EFAULT},
-	{"null-param", &buf, &size, NULL, 0, EFAULT},
-	{"zero-size", &buf, &zero_size, "", 0, ENOEXEC},
-	{"invalid_param", &buf, &size, "status=invalid", 0, EINVAL},
-	{"no-perm", &buf, &size, "", 1, EPERM},
-	{"module-exists", &buf, &size, "", 0, EEXIST},
+	{"NULL-buffer", &null_buf, &size, "", 0, EFAULT, EFAULT},
+	{"faulty-buffer", &faulty_buf, &size, "", 0, EFAULT, EFAULT},
+	{"null-param", &buf, &size, NULL, 0, EFAULT, EPERM},
+	{"zero-size", &buf, &zero_size, "", 0, ENOEXEC, ENOEXEC},
+	{"invalid_param", &buf, &size, "status=invalid", 0, EINVAL, EPERM},
+	{"no-perm", &buf, &size, "", 1, EPERM, EPERM},
+	{"module-exists", &buf, &size, "", 0, EEXIST, EPERM},
 };
 
 static void setup(void)
@@ -51,6 +53,7 @@ static void setup(void)
 
 	tst_module_exists(MODULE_NAME, NULL);
 
+	kernel_lockdown = tst_lockdown_enabled();
 	fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC);
 	SAFE_FSTAT(fd, &sb);
 	size = sb.st_size;
@@ -68,10 +71,18 @@ static void run(unsigned int n)
 		tst_cap_action(&cap_drop);
 
 	/* Insert module twice */
-	if (tc->exp_errno == EEXIST)
+	if (tc->exp_errno == EEXIST) {
+		if (kernel_lockdown) {
+			tst_res(TCONF, "Kernel is locked down, skipping %s",
+				tc->name);
+			return;
+		}
+
 		tst_module_load(MODULE_NAME, NULL);
+	}
 
-	TST_EXP_FAIL(init_module(*tc->buf, *tc->size, tc->param), tc->exp_errno,
+	TST_EXP_FAIL(init_module(*tc->buf, *tc->size, tc->param),
+		     kernel_lockdown ? tc->lockdown_errno : tc->exp_errno,
 		     "TestName: %s", tc->name);
 
 	if (tc->exp_errno == EEXIST)
-- 
2.32.0

[LTP] [PATCH 3/3] Add lockdown checks to init_module* and finit_module* tests

[Cyril Hrubis]

Hi!
> +static void lockdown_setup(struct tcase *tc)
> +{
> +	if (kernel_lockdown)
> +		tc->exp_errno = EPERM;
> +}
> +
>  static struct tcase tcases[] = {
>  	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, bad_fd_setup},
>  	{"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
> -	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT, NULL},
> -	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
> +	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT,
> +		lockdown_setup},
> +	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0,
> +		EINVAL, lockdown_setup},
>  	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, NULL},
>  	{"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, NULL},
>  	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, NULL},

I'm slightly afraid that the order of checks may change over the time
and we will get EPERM in all these cases, but maybe I'm just overly
cautious. Other than this the code looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] Add lockdown checks to init_module* and finit_module* tests

[Martin Doucha]

On 20. 07. 21 14:02, Cyril Hrubis wrote:
> Hi!
>> +static void lockdown_setup(struct tcase *tc)
>> +{
>> +	if (kernel_lockdown)
>> +		tc->exp_errno = EPERM;
>> +}
>> +
>>  static struct tcase tcases[] = {
>>  	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, bad_fd_setup},
>>  	{"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
>> -	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT, NULL},
>> -	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, NULL},
>> +	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT,
>> +		lockdown_setup},
>> +	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0,
>> +		EINVAL, lockdown_setup},
>>  	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, NULL},
>>  	{"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, NULL},
>>  	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, NULL},
> 
> I'm slightly afraid that the order of checks may change over the time
> and we will get EPERM in all these cases, but maybe I'm just overly
> cautious. Other than this the code looks good.

I don't think we need to worry about that. With root privileges, the
EPERM error is returned when a kernel module does not have a valid
signature. How would something that is not even a valid kernel module in
the first place fail that check?

The only subtests that actually try to load a valid kernel module are
null-param, invalid-param and module-exists. All three of them now
handle lockdown correctly.

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

[LTP] [PATCH 3/3] Add lockdown checks to init_module* and finit_module* tests

[Cyril Hrubis]

Hi!
> > I'm slightly afraid that the order of checks may change over the time
> > and we will get EPERM in all these cases, but maybe I'm just overly
> > cautious. Other than this the code looks good.
> 
> I don't think we need to worry about that. With root privileges, the
> EPERM error is returned when a kernel module does not have a valid
> signature. How would something that is not even a valid kernel module in
> the first place fail that check?
>
> The only subtests that actually try to load a valid kernel module are
> null-param, invalid-param and module-exists. All three of them now
> handle lockdown correctly.

Right, we have to be able to read the signature in order to produce
EPERM and the same for the init_module() there has to be a pointer to
a module data that kernel can check the signature from.

Patch pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] Add lockdown checks to init_module* and finit_module* tests

[Petr Vorel]

Hi Martin,

thanks a lot for fixing this.

Kind regards,
Petr

[LTP] [PATCH 1/3] Add skip_in_lockdown flag to struct tst_test

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz