All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
@ 2020-07-20 19:49 Erico Nunes
  2020-07-20 19:49 ` [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down Erico Nunes
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Erico Nunes @ 2020-07-20 19:49 UTC (permalink / raw)
  To: ltp

Some syscalls are not available if the kernel is booted using the
'lockdown' feature. That can cause some tests to report fail, showing
a message like:

  Lockdown: iopl01: iopl is restricted; see man kernel_lockdown.7

This patch adds a function that can be used by tests to check for this
case, so tests can be skipped rather than reporting a test failure.

Signed-off-by: Erico Nunes <ernunes@redhat.com>
---
 include/tst_lockdown.h |  8 ++++++++
 include/tst_test.h     |  1 +
 lib/tst_lockdown.c     | 28 ++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)
 create mode 100644 include/tst_lockdown.h
 create mode 100644 lib/tst_lockdown.c

diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
new file mode 100644
index 000000000..8db26d943
--- /dev/null
+++ b/include/tst_lockdown.h
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#ifndef TST_LOCKDOWN_H
+#define TST_LOCKDOWN_H
+
+void tst_lockdown_skip(void);
+
+#endif /* TST_LOCKDOWN_H */
diff --git a/include/tst_test.h b/include/tst_test.h
index b84f7b9dd..b02de4597 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -40,6 +40,7 @@
 #include "tst_hugepage.h"
 #include "tst_assert.h"
 #include "tst_cgroup.h"
+#include "tst_lockdown.h"
 
 /*
  * Reports testcase result.
diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
new file mode 100644
index 000000000..d57a6bdf3
--- /dev/null
+++ b/lib/tst_lockdown.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#define TST_NO_DEFAULT_MAIN
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mount.h>
+
+#include "tst_test.h"
+#include "tst_safe_macros.h"
+#include "tst_safe_stdio.h"
+#include "tst_lockdown.h"
+
+void tst_lockdown_skip(void)
+{
+	char line[BUFSIZ];
+	FILE *file;
+
+	if (access("/sys/kernel/security/lockdown", F_OK) != 0)
+		return;
+
+	file = SAFE_FOPEN("/sys/kernel/security/lockdown", "r");
+	fgets(line, sizeof(line), file);
+	SAFE_FCLOSE(file);
+
+	if (strstr(line, "[none]") == NULL)
+		tst_brk(TCONF, "Kernel is locked down, skip this test.");
+}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down
  2020-07-20 19:49 [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Erico Nunes
@ 2020-07-20 19:49 ` Erico Nunes
  2020-07-21 15:26   ` Cyril Hrubis
  2020-07-20 19:49 ` [LTP] [PATCH 3/3] iopl01: " Erico Nunes
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Erico Nunes @ 2020-07-20 19:49 UTC (permalink / raw)
  To: ltp

ioperm is restricted under kernel lockdown.

Signed-off-by: Erico Nunes <ernunes@redhat.com>
---
 testcases/kernel/syscalls/ioperm/ioperm01.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/testcases/kernel/syscalls/ioperm/ioperm01.c b/testcases/kernel/syscalls/ioperm/ioperm01.c
index 4c5c0e6ea..d1d633b20 100644
--- a/testcases/kernel/syscalls/ioperm/ioperm01.c
+++ b/testcases/kernel/syscalls/ioperm/ioperm01.c
@@ -42,6 +42,9 @@ static void verify_ioperm(void)
 
 static void setup(void)
 {
+	/* ioperm is restricted under kernel lockdown. */
+	tst_lockdown_skip();
+
 	/*
 	 * The value of IO_BITMAP_BITS (include/asm-i386/processor.h) changed
 	 * from kernel 2.6.8 to permit 16-bits ioperm
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 3/3] iopl01: skip test if kernel is locked down
  2020-07-20 19:49 [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Erico Nunes
  2020-07-20 19:49 ` [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down Erico Nunes
@ 2020-07-20 19:49 ` Erico Nunes
  2020-07-21 15:29   ` Cyril Hrubis
  2020-07-21  7:46 ` [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Li Wang
  2020-07-21 15:26 ` Cyril Hrubis
  3 siblings, 1 reply; 14+ messages in thread
From: Erico Nunes @ 2020-07-20 19:49 UTC (permalink / raw)
  To: ltp

iopl is restricted under kernel lockdown.

Signed-off-by: Erico Nunes <ernunes@redhat.com>
---
 testcases/kernel/syscalls/iopl/iopl01.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/testcases/kernel/syscalls/iopl/iopl01.c b/testcases/kernel/syscalls/iopl/iopl01.c
index edf586cd1..4237d3f10 100644
--- a/testcases/kernel/syscalls/iopl/iopl01.c
+++ b/testcases/kernel/syscalls/iopl/iopl01.c
@@ -42,6 +42,12 @@ static void verify_iopl(void)
 	}
 }
 
+static void setup(void)
+{
+	/* iopl is restricted under kernel lockdown. */
+	tst_lockdown_skip();
+}
+
 static void cleanup(void)
 {
 	/*
@@ -54,6 +60,7 @@ static void cleanup(void)
 static struct tst_test test = {
 	.test_all = verify_iopl,
 	.needs_root = 1,
+	.setup = setup,
 	.cleanup = cleanup,
 };
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-20 19:49 [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Erico Nunes
  2020-07-20 19:49 ` [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down Erico Nunes
  2020-07-20 19:49 ` [LTP] [PATCH 3/3] iopl01: " Erico Nunes
@ 2020-07-21  7:46 ` Li Wang
  2020-07-21  8:57   ` Erico Nunes
  2020-07-21 15:26 ` Cyril Hrubis
  3 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2020-07-21  7:46 UTC (permalink / raw)
  To: ltp

Hi Erico,

Thanks for working on this fix. Comments as below:

On Tue, Jul 21, 2020 at 3:50 AM Erico Nunes <ernunes@redhat.com> wrote:

> Some syscalls are not available if the kernel is booted using the
> 'lockdown' feature. That can cause some tests to report fail, showing
> a message like:
>
>   Lockdown: iopl01: iopl is restricted; see man kernel_lockdown.7
>
> This patch adds a function that can be used by tests to check for this
> case, so tests can be skipped rather than reporting a test failure.
>
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  include/tst_lockdown.h |  8 ++++++++
>  include/tst_test.h     |  1 +
>  lib/tst_lockdown.c     | 28 ++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+)
>  create mode 100644 include/tst_lockdown.h
>  create mode 100644 lib/tst_lockdown.c
>
> diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
> new file mode 100644
> index 000000000..8db26d943
> --- /dev/null
> +++ b/include/tst_lockdown.h
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#ifndef TST_LOCKDOWN_H
> +#define TST_LOCKDOWN_H
> +
> +void tst_lockdown_skip(void);
> +
> +#endif /* TST_LOCKDOWN_H */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index b84f7b9dd..b02de4597 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -40,6 +40,7 @@
>  #include "tst_hugepage.h"
>  #include "tst_assert.h"
>  #include "tst_cgroup.h"
> +#include "tst_lockdown.h"
>
>  /*
>   * Reports testcase result.
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> new file mode 100644
> index 000000000..d57a6bdf3
> --- /dev/null
> +++ b/lib/tst_lockdown.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#define TST_NO_DEFAULT_MAIN
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/mount.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "tst_safe_stdio.h"
> +#include "tst_lockdown.h"
> +
> +void tst_lockdown_skip(void)
>

Maybe renaming the function to tst_lockdown_enabled() is better? Then we
can return 1 if confirm kernel under lockdown mode otherwise 0.

+{
> +       char line[BUFSIZ];
> +       FILE *file;
> +
> +       if (access("/sys/kernel/security/lockdown", F_OK) != 0)
>

After thinking over, I guess it's not enough to only check /sys/../lockdown
file. Seems we need to consider the situation that system without
supporting this file?

i.e.
  Test on RHEL8 (no /sys/../lockdown file) with kernel parameter "lockdown"
and got the restriction error too.

# cat /proc/cmdline
BOOT_IMAGE=(hd0,msdos1)/vmlinuz-4.18.0-226.el8.x86_64
root=/dev/mapper/rhel_bootp--73--3--209-root ro console=ttyS0,115200 ...
 lockdown

# ll /sys/kernel/security/lockdown
ls: cannot access '/sys/kernel/security/lockdown': No such file or directory

# ./iopl01
...
iopl01.c:37: FAIL: iopl() failed for level 1, errno=1 : EPERM: EPERM (1)
iopl01.c:37: FAIL: iopl() failed for level 2, errno=1 : EPERM: EPERM (1)



> +               return;
> +
> +       file = SAFE_FOPEN("/sys/kernel/security/lockdown", "r");
> +       fgets(line, sizeof(line), file);
> +       SAFE_FCLOSE(file);
> +
> +       if (strstr(line, "[none]") == NULL)
> +               tst_brk(TCONF, "Kernel is locked down, skip this test.");
> +}
> --
> 2.26.2
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200721/c9de85e0/attachment.htm>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-21  7:46 ` [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Li Wang
@ 2020-07-21  8:57   ` Erico Nunes
  2020-07-21 13:19     ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Erico Nunes @ 2020-07-21  8:57 UTC (permalink / raw)
  To: ltp

Thanks for the review. I'll address other comments soon, just an initial
note below:

On 7/21/20 9:46 AM, Li Wang wrote:
> Maybe renaming the function to tst_lockdown_enabled() is better? Then we
> can return?1 if confirm kernel under lockdown mode otherwise 0.
> 
>     +{
>     +? ? ? ?char line[BUFSIZ];
>     +? ? ? ?FILE *file;
>     +
>     +? ? ? ?if (access("/sys/kernel/security/lockdown", F_OK) != 0)
> 
> 
> After thinking over, I guess it's not enough to only check
> /sys/../lockdown file. Seems we need to consider the situation that
> system without supporting this file??
> 
> i.e.?
> ? Test on RHEL8 (no /sys/../lockdown file) with kernel parameter
> "lockdown" and got the restriction error too.
> 
> # cat /proc/cmdline?
> BOOT_IMAGE=(hd0,msdos1)/vmlinuz-4.18.0-226.el8.x86_64
> root=/dev/mapper/rhel_bootp--73--3--209-root ro console=ttyS0,115200
> ...?lockdown
> ? ??
> # ll /sys/kernel/security/lockdown
> ls: cannot access '/sys/kernel/security/lockdown': No such file or directory

To my understanding, the parameter to enable lockdown through kenrel
parameters is "lockdown={integrity|confidentiality}", not just
"lockdown", at least on upstream kernels:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aefcf2f4b58155d27340ba5f9ddbe9513da8286d

If /sys/kernel/security/lockdown doesn't exist, I'm not sure there is
much we can do easily, or that is worth doing now. I think it is ok to
fall back and fail like it has been happening since the feature was
merged upstream.
I can't see a tweak that would enable the feature but not the sysfs file
in the kernel source. Maybe that kernel only had partial support?

Erico


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-21  8:57   ` Erico Nunes
@ 2020-07-21 13:19     ` Li Wang
  2020-07-22 15:52       ` Erico Nunes
  0 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2020-07-21 13:19 UTC (permalink / raw)
  To: ltp

Erico,

On Tue, Jul 21, 2020 at 4:57 PM Erico Nunes <ernunes@redhat.com> wrote:

> ...
>
> > Maybe renaming the function to tst_lockdown_enabled() is better? Then we
> > can return 1 if confirm kernel under lockdown mode otherwise 0.
>

How do you think about this suggestion? ^^

Another reason to name it as tst_lockdown_enabled() is, we can give more
flexible
to test case, because not all tests need a simple skip in lockdown mode(in
future).

i.e.
if (tst_lockdown_enabled()) {
   // skip or not,
   // do what they wanted in this mode
}


> After thinking over, I guess it's not enough to only check
> > /sys/../lockdown file. Seems we need to consider the situation that
> > system without supporting this file?
> >
> > i.e.
> >   Test on RHEL8 (no /sys/../lockdown file) with kernel parameter
> > "lockdown" and got the restriction error too.
> >
> > # cat /proc/cmdline
> > BOOT_IMAGE=(hd0,msdos1)/vmlinuz-4.18.0-226.el8.x86_64
> > root=/dev/mapper/rhel_bootp--73--3--209-root ro console=ttyS0,115200
> > ... lockdown
> >
> > # ll /sys/kernel/security/lockdown
> > ls: cannot access '/sys/kernel/security/lockdown': No such file or
> directory
>
> To my understanding, the parameter to enable lockdown through kenrel
> parameters is "lockdown={integrity|confidentiality}", not just
> "lockdown", at least on upstream kernels:
>

Good to know this.


>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aefcf2f4b58155d27340ba5f9ddbe9513da8286d
>


>
> If /sys/kernel/security/lockdown doesn't exist, I'm not sure there is
> much we can do easily, or that is worth doing now. I think it is ok to
> fall back and fail like it has been happening since the feature was
> merged upstream.
>

Yes, it looks a bit tricky.


> I can't see a tweak that would enable the feature but not the sysfs file
> in the kernel source. Maybe that kernel only had partial support?
>

Seems you're right, there are many differences between mainline-kernel
and some distros in lockdown code. The reason that some distribution
(i.e RHEL, Ubuntu) partly customizes the LSM feature, it does not support
lockdown features completely so far.

But one point we're sure is that the /sys/kernel/../lockdown file was
introduced from kernel-v5.4.

So maybe we could simply do detect the /sys/kernel/../loackdown file as
your patch,
but adding an extra warning print when test failed on older than
kernel-v5.4.

Or, if others can provide a better way I'd happy to hear.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200721/88b05af2/attachment.htm>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-20 19:49 [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Erico Nunes
                   ` (2 preceding siblings ...)
  2020-07-21  7:46 ` [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Li Wang
@ 2020-07-21 15:26 ` Cyril Hrubis
  2020-07-22 15:52   ` Erico Nunes
  3 siblings, 1 reply; 14+ messages in thread
From: Cyril Hrubis @ 2020-07-21 15:26 UTC (permalink / raw)
  To: ltp

Hi!
> Some syscalls are not available if the kernel is booted using the
> 'lockdown' feature. That can cause some tests to report fail, showing
> a message like:
> 
>   Lockdown: iopl01: iopl is restricted; see man kernel_lockdown.7
> 
> This patch adds a function that can be used by tests to check for this
> case, so tests can be skipped rather than reporting a test failure.
> 
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  include/tst_lockdown.h |  8 ++++++++
>  include/tst_test.h     |  1 +
>  lib/tst_lockdown.c     | 28 ++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+)
>  create mode 100644 include/tst_lockdown.h
>  create mode 100644 lib/tst_lockdown.c
> 
> diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
> new file mode 100644
> index 000000000..8db26d943
> --- /dev/null
> +++ b/include/tst_lockdown.h
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#ifndef TST_LOCKDOWN_H
> +#define TST_LOCKDOWN_H
> +
> +void tst_lockdown_skip(void);
> +
> +#endif /* TST_LOCKDOWN_H */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index b84f7b9dd..b02de4597 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -40,6 +40,7 @@
>  #include "tst_hugepage.h"
>  #include "tst_assert.h"
>  #include "tst_cgroup.h"
> +#include "tst_lockdown.h"
>  
>  /*
>   * Reports testcase result.
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> new file mode 100644
> index 000000000..d57a6bdf3
> --- /dev/null
> +++ b/lib/tst_lockdown.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#define TST_NO_DEFAULT_MAIN
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/mount.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "tst_safe_stdio.h"
> +#include "tst_lockdown.h"
> +
> +void tst_lockdown_skip(void)
> +{
> +	char line[BUFSIZ];
> +	FILE *file;
> +
> +	if (access("/sys/kernel/security/lockdown", F_OK) != 0)
> +		return;
> +
> +	file = SAFE_FOPEN("/sys/kernel/security/lockdown", "r");
> +	fgets(line, sizeof(line), file);

The compiler complains that we haven't checked the return value here I
guess that we can silence it with:

	if (!fgets(line, sizeof(line), file)
		return;

> +	SAFE_FCLOSE(file);
> +
> +	if (strstr(line, "[none]") == NULL)
> +		tst_brk(TCONF, "Kernel is locked down, skip this test.");
> +}
> -- 
> 2.26.2
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down
  2020-07-20 19:49 ` [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down Erico Nunes
@ 2020-07-21 15:26   ` Cyril Hrubis
  2020-07-22 15:52     ` Erico Nunes
  0 siblings, 1 reply; 14+ messages in thread
From: Cyril Hrubis @ 2020-07-21 15:26 UTC (permalink / raw)
  To: ltp

Hi!
> ioperm is restricted under kernel lockdown.
> 
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  testcases/kernel/syscalls/ioperm/ioperm01.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/ioperm/ioperm01.c b/testcases/kernel/syscalls/ioperm/ioperm01.c
> index 4c5c0e6ea..d1d633b20 100644
> --- a/testcases/kernel/syscalls/ioperm/ioperm01.c
> +++ b/testcases/kernel/syscalls/ioperm/ioperm01.c
> @@ -42,6 +42,9 @@ static void verify_ioperm(void)
>  
>  static void setup(void)
>  {
> +	/* ioperm is restricted under kernel lockdown. */
> +	tst_lockdown_skip();
> +
>  	/*
>  	 * The value of IO_BITMAP_BITS (include/asm-i386/processor.h) changed
>  	 * from kernel 2.6.8 to permit 16-bits ioperm

This looks good, however shouldn't we as well add a third ioperm test
that checks that the call fails for root when lockdown is enabled?

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 3/3] iopl01: skip test if kernel is locked down
  2020-07-20 19:49 ` [LTP] [PATCH 3/3] iopl01: " Erico Nunes
@ 2020-07-21 15:29   ` Cyril Hrubis
  0 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2020-07-21 15:29 UTC (permalink / raw)
  To: ltp

Hi!
> iopl is restricted under kernel lockdown.
> 
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  testcases/kernel/syscalls/iopl/iopl01.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/iopl/iopl01.c b/testcases/kernel/syscalls/iopl/iopl01.c
> index edf586cd1..4237d3f10 100644
> --- a/testcases/kernel/syscalls/iopl/iopl01.c
> +++ b/testcases/kernel/syscalls/iopl/iopl01.c
> @@ -42,6 +42,12 @@ static void verify_iopl(void)
>  	}
>  }
>  
> +static void setup(void)
> +{
> +	/* iopl is restricted under kernel lockdown. */
> +	tst_lockdown_skip();
> +}
> +
>  static void cleanup(void)
>  {
>  	/*
> @@ -54,6 +60,7 @@ static void cleanup(void)
>  static struct tst_test test = {
>  	.test_all = verify_iopl,
>  	.needs_root = 1,
> +	.setup = setup,
>  	.cleanup = cleanup,
>  };

Here as well, shouldn't we add iopl03?

Or at least change the library so that we have a function that returns
if kernel is locked or not so that someone can write such test.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-21 15:26 ` Cyril Hrubis
@ 2020-07-22 15:52   ` Erico Nunes
  0 siblings, 0 replies; 14+ messages in thread
From: Erico Nunes @ 2020-07-22 15:52 UTC (permalink / raw)
  To: ltp

On 7/21/20 5:26 PM, Cyril Hrubis wrote:
>> +void tst_lockdown_skip(void)
>> +{
>> +	char line[BUFSIZ];
>> +	FILE *file;
>> +
>> +	if (access("/sys/kernel/security/lockdown", F_OK) != 0)
>> +		return;
>> +
>> +	file = SAFE_FOPEN("/sys/kernel/security/lockdown", "r");
>> +	fgets(line, sizeof(line), file);
> 
> The compiler complains that we haven't checked the return value here I
> guess that we can silence it with:
> 
> 	if (!fgets(line, sizeof(line), file)
> 		return;
> 

Thanks, will fix in v2.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down
  2020-07-21 15:26   ` Cyril Hrubis
@ 2020-07-22 15:52     ` Erico Nunes
  0 siblings, 0 replies; 14+ messages in thread
From: Erico Nunes @ 2020-07-22 15:52 UTC (permalink / raw)
  To: ltp

On 7/21/20 5:26 PM, Cyril Hrubis wrote:
> Hi!
>> ioperm is restricted under kernel lockdown.
>>
>> Signed-off-by: Erico Nunes <ernunes@redhat.com>
>> ---
>>  testcases/kernel/syscalls/ioperm/ioperm01.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/testcases/kernel/syscalls/ioperm/ioperm01.c b/testcases/kernel/syscalls/ioperm/ioperm01.c
>> index 4c5c0e6ea..d1d633b20 100644
>> --- a/testcases/kernel/syscalls/ioperm/ioperm01.c
>> +++ b/testcases/kernel/syscalls/ioperm/ioperm01.c
>> @@ -42,6 +42,9 @@ static void verify_ioperm(void)
>>  
>>  static void setup(void)
>>  {
>> +	/* ioperm is restricted under kernel lockdown. */
>> +	tst_lockdown_skip();
>> +
>>  	/*
>>  	 * The value of IO_BITMAP_BITS (include/asm-i386/processor.h) changed
>>  	 * from kernel 2.6.8 to permit 16-bits ioperm
> 
> This looks good, however shouldn't we as well add a third ioperm test
> that checks that the call fails for root when lockdown is enabled?
> 

Good point. I think it is a good idea, but can be done in a separate
patchset.
There are many other things that can be covered together with that by
new tests considering the recent lockdown feature (enum lockdown_reason
seems to provide a good list and starting point).

Erico


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-21 13:19     ` Li Wang
@ 2020-07-22 15:52       ` Erico Nunes
  2020-07-22 15:58         ` Cyril Hrubis
  2020-07-23  7:51         ` Li Wang
  0 siblings, 2 replies; 14+ messages in thread
From: Erico Nunes @ 2020-07-22 15:52 UTC (permalink / raw)
  To: ltp

On 7/21/20 3:19 PM, Li Wang wrote:
> On Tue, Jul 21, 2020 at 4:57 PM Erico Nunes <ernunes@redhat.com
> <mailto:ernunes@redhat.com>> wrote:
> 
>     ...
> 
>     > Maybe renaming the function to tst_lockdown_enabled() is better?
>     Then we
>     > can return?1 if confirm kernel under lockdown mode otherwise 0.
> 
> 
> How do you think about this suggestion? ^^
> 
> Another reason?to name it as tst_lockdown_enabled() is, we can give more
> flexible
> to test case, because?not all tests need a simple skip in lockdown
> mode(in future).
> 
> i.e.
> if (tst_lockdown_enabled()) {
> ? ?// skip or not,
> ? ?// do what they wanted in this mode
> }

I like this suggestion, I'll send v2 with this.

>     If /sys/kernel/security/lockdown doesn't exist, I'm not sure there is
>     much we can do easily, or that is worth doing now. I think it is ok to
>     fall back and fail like it has been happening since the feature was
>     merged upstream.
> 
> 
> Yes, it looks a bit tricky.
> ?
> 
>     I can't see a tweak that would enable the feature but not the sysfs file
>     in the kernel source. Maybe that kernel only had partial support?
> 
> 
> Seems you're right, there are many differences between mainline-kernel
> and some distros in lockdown code. The reason that some distribution
> (i.e RHEL, Ubuntu) partly customizes the LSM feature,?it does not support
> lockdown features completely so far.
> 
> But one point we're sure is that the /sys/kernel/../lockdown file was
> introduced from kernel-v5.4.
> 
> So maybe we could simply do detect the /sys/kernel/../loackdown file as
> your patch,
> but adding an extra warning print when test failed on older than
> kernel-v5.4.

I like the idea of the warning. The only thing to consider is that the
warning would also show up on all old kernels that don't even support
lockdown and then don't have the file. So would you suggest this message
to be something like a tst_res(TWARN, ...) or TINFO or some other less
noisy way?

I also thought about limiting to some kernel version but that wouldn't
work with distribution kernels like RHEL which have an earlier version
number but also have the feature...

Erico


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-22 15:52       ` Erico Nunes
@ 2020-07-22 15:58         ` Cyril Hrubis
  2020-07-23  7:51         ` Li Wang
  1 sibling, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2020-07-22 15:58 UTC (permalink / raw)
  To: ltp

Hi!
> > So maybe we could simply do detect the /sys/kernel/../loackdown file as
> > your patch,
> > but adding an extra warning print when test failed on older than
> > kernel-v5.4.
> 
> I like the idea of the warning. The only thing to consider is that the
> warning would also show up on all old kernels that don't even support
> lockdown and then don't have the file. So would you suggest this message
> to be something like a tst_res(TWARN, ...) or TINFO or some other less
> noisy way?

TWARN will cause the test to exit with non-zero status, which will
probably show up as a failure in some environments, so I would go for
TINFO.

> I also thought about limiting to some kernel version but that wouldn't
> work with distribution kernels like RHEL which have an earlier version
> number but also have the feature...

We also have an interface to match different kernel versions per
distribution, have a look at tst_kern_exv structure in inotify04.c
testcase.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown
  2020-07-22 15:52       ` Erico Nunes
  2020-07-22 15:58         ` Cyril Hrubis
@ 2020-07-23  7:51         ` Li Wang
  1 sibling, 0 replies; 14+ messages in thread
From: Li Wang @ 2020-07-23  7:51 UTC (permalink / raw)
  To: ltp

On Wed, Jul 22, 2020 at 11:52 PM Erico Nunes <ernunes@redhat.com> wrote:

> ...
> > So maybe we could simply do detect the /sys/kernel/../loackdown file as
> > your patch,
> > but adding an extra warning print when test failed on older than
> > kernel-v5.4.
>
> I like the idea of the warning. The only thing to consider is that the

warning would also show up on all old kernels that don't even support


lockdown and then don't have the file. So would you suggest this message
> to be something like a tst_res(TWARN, ...) or TINFO or some other less
> noisy way?
>
Thanks, but I did not suggest to show the warning in any system without the
lockdown file. I mean if test getting FAIL on the no 'lockdown' file
system, then could consider throwing a warning as a hint.
And this could be achieved in the test case but not the library.

For indicating the 'lockdown' file exist or no-exist, the 'TINFO' is enough.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200723/2a96b60d/attachment-0001.htm>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-07-23  7:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 19:49 [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Erico Nunes
2020-07-20 19:49 ` [LTP] [PATCH 2/3] ioperm01: skip test if kernel is locked down Erico Nunes
2020-07-21 15:26   ` Cyril Hrubis
2020-07-22 15:52     ` Erico Nunes
2020-07-20 19:49 ` [LTP] [PATCH 3/3] iopl01: " Erico Nunes
2020-07-21 15:29   ` Cyril Hrubis
2020-07-21  7:46 ` [LTP] [PATCH 1/3] lib: add function to check for kernel lockdown Li Wang
2020-07-21  8:57   ` Erico Nunes
2020-07-21 13:19     ` Li Wang
2020-07-22 15:52       ` Erico Nunes
2020-07-22 15:58         ` Cyril Hrubis
2020-07-23  7:51         ` Li Wang
2020-07-21 15:26 ` Cyril Hrubis
2020-07-22 15:52   ` Erico Nunes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.