[LTP] [PATCH] lib: lockdown: Report lockdown as disabled on missing sysfs

[LTP] [PATCH] lib: lockdown: Report lockdown as disabled on missing sysfs

[Cyril Hrubis]

We currently report -1 when secure boot sysfs file is not present which
is later interpreted as secure boot enabled. This causes regression in
*_module sycall tests executed on systems when secureboot is not
compiled-in or supported at all.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/tst_lockdown.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
index 38d830886..7613092ec 100644
--- a/lib/tst_lockdown.c
+++ b/lib/tst_lockdown.c
@@ -29,7 +29,7 @@ int tst_secureboot_enabled(void)
 
 	if (access(SECUREBOOT_VAR, F_OK)) {
 		tst_res(TINFO, "SecureBoot sysfs file not available");
-		return -1;
+		return 0;
 	}
 
 	fd = open(SECUREBOOT_VAR, O_RDONLY);
-- 
2.41.0


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

Re: [LTP] [PATCH] lib: lockdown: Report lockdown as disabled on missing sysfs

[Martin Doucha]

On 20. 09. 23 17:44, Cyril Hrubis wrote:
> We currently report -1 when secure boot sysfs file is not present which
> is later interpreted as secure boot enabled. This causes regression in
> *_module sycall tests executed on systems when secureboot is not
> compiled-in or supported at all.

That's incorrect usage then. The tests should check 
tst_secureboot_enabled() > 0 instead. I think it will be useful to know 
whether the function found that secureboot is disabled, or could not 
check at all. We should just document it better.

> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>   lib/tst_lockdown.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> index 38d830886..7613092ec 100644
> --- a/lib/tst_lockdown.c
> +++ b/lib/tst_lockdown.c
> @@ -29,7 +29,7 @@ int tst_secureboot_enabled(void)
>   
>   	if (access(SECUREBOOT_VAR, F_OK)) {
>   		tst_res(TINFO, "SecureBoot sysfs file not available");
> -		return -1;
> +		return 0;
>   	}
>   
>   	fd = open(SECUREBOOT_VAR, O_RDONLY);

-- 
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] lib: lockdown: Report lockdown as disabled on missing sysfs

[Cyril Hrubis]

Hi!
> > We currently report -1 when secure boot sysfs file is not present which
> > is later interpreted as secure boot enabled. This causes regression in
> > *_module sycall tests executed on systems when secureboot is not
> > compiled-in or supported at all.
> 
> That's incorrect usage then. The tests should check 
> tst_secureboot_enabled() > 0 instead. I think it will be useful to know 
> whether the function found that secureboot is disabled, or could not 
> check at all. We should just document it better.

Yes, the functions do not seem to have any documentation.

So I guess that we need:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2e58cad33..e2c195645 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1163,10 +1163,10 @@ static void do_setup(int argc, char *argv[])
        if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
                tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);

-       if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
+       if (tst_test->skip_in_lockdown && tst_lockdown_enabled() > 0)
                tst_brk(TCONF, "Kernel is locked down, skipping test");

-       if (tst_test->skip_in_secureboot && tst_secureboot_enabled())
+       if (tst_test->skip_in_secureboot && tst_secureboot_enabled() > 0)
                tst_brk(TCONF, "SecureBoot enabled, skipping test");

        if (tst_test->skip_in_compat && TST_ABI != tst_kernel_bits())

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] lib: lockdown: Report lockdown as disabled on missing sysfs

[Martin Doucha]

Hi,

On 20. 09. 23 20:17, Cyril Hrubis wrote:
> Hi!
>>> We currently report -1 when secure boot sysfs file is not present which
>>> is later interpreted as secure boot enabled. This causes regression in
>>> *_module sycall tests executed on systems when secureboot is not
>>> compiled-in or supported at all.
>>
>> That's incorrect usage then. The tests should check
>> tst_secureboot_enabled() > 0 instead. I think it will be useful to know
>> whether the function found that secureboot is disabled, or could not
>> check at all. We should just document it better.
> 
> Yes, the functions do not seem to have any documentation.
> 
> So I guess that we need:
> 
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2e58cad33..e2c195645 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1163,10 +1163,10 @@ static void do_setup(int argc, char *argv[])
>          if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>                  tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
> 
> -       if (tst_test->skip_in_lockdown && tst_lockdown_enabled())
> +       if (tst_test->skip_in_lockdown && tst_lockdown_enabled() > 0)
>                  tst_brk(TCONF, "Kernel is locked down, skipping test");
> 
> -       if (tst_test->skip_in_secureboot && tst_secureboot_enabled())
> +       if (tst_test->skip_in_secureboot && tst_secureboot_enabled() > 0)
>                  tst_brk(TCONF, "SecureBoot enabled, skipping test");
> 
>          if (tst_test->skip_in_compat && TST_ABI != tst_kernel_bits())
> 

Yes, this is the correct fix.

-- 
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