[LTP] [PATCH V2] lib/tst_lockdown.c: Add PPC64 architecture support

[Martin Doucha]

Hi,

On 05. 09. 23 8:04, Nageswara R Sastry wrote:
> Add PPC64 architecture support to the lockdown library.
> 
> Signed-off-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> ---
> v2:
>   - Group all the constant definitions together (Cyril Hrubis)
>   - Reduce the number of variables (Martin Doucha)
> ---
>   lib/tst_lockdown.c | 35 +++++++++++++++++++++++------------
>   1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> index 9086eba36..ea71f6753 100644
> --- a/lib/tst_lockdown.c
> +++ b/lib/tst_lockdown.c
> @@ -14,33 +14,42 @@
>   #include "tst_lockdown.h"
>   #include "tst_private.h"
>   
> -#define EFIVAR_SECUREBOOT "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c"
> -
> +#if defined(__powerpc64__) || defined(__ppc64__)
> +# define KERNEL_CONFIG1 "CONFIG_SECURITY_LOCKDOWN_LSM"
> +# define KERNEL_CONFIG2 "CONFIG_SECURITY_LOCKDOWN_LSM_EARLY"
> +# define SECUREBOOT_VAR "/proc/device-tree/ibm,secure-boot"
> +# define VAR_DATA_SIZE 4
> +#else
> +# define KERNEL_CONFIG1 "CONFIG_EFI_SECURE_BOOT_LOCK_DOWN"
> +# define KERNEL_CONFIG2 "CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT"
> +# define SECUREBOOT_VAR "/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c"
> +# define VAR_DATA_SIZE 5
> +#endif
>   int tst_secureboot_enabled(void)
>   {
>   	int fd;
>   	char data[5];
>   
> -	if (access(EFIVAR_SECUREBOOT, F_OK)) {
> -		tst_res(TINFO, "Efivar FS not available");
> +	if (access(SECUREBOOT_VAR, F_OK)) {
> +		tst_res(TINFO, "SecureBoot sysfs file not available");
>   		return -1;
>   	}
>   
> -	fd = open(EFIVAR_SECUREBOOT, O_RDONLY);
> +	fd = open(SECUREBOOT_VAR, O_RDONLY);
>   
>   	if (fd == -1) {
>   		tst_res(TINFO | TERRNO,
> -			"Cannot open SecureBoot Efivar sysfile");
> +			"Cannot open SecureBoot file");
>   		return -1;
>   	} else if (fd < 0) {
>   		tst_brk(TBROK | TERRNO, "Invalid open() return value %d", fd);
>   		return -1;
>   	}
> -
> -	SAFE_READ(1, fd, data, 5);
> +	SAFE_READ(1, fd, data, VAR_DATA_SIZE);
>   	SAFE_CLOSE(fd);
> -	tst_res(TINFO, "SecureBoot: %s", data[4] ? "on" : "off");
> -	return data[4];
> +
> +	tst_res(TINFO, "SecureBoot: %s", data[VAR_DATA_SIZE - 1] ? "on" : "off");
> +	return data[VAR_DATA_SIZE - 1];
>   }
>   
>   int tst_lockdown_enabled(void)
> @@ -51,9 +60,11 @@ int tst_lockdown_enabled(void)
>   
>   	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") == 'y';
> -		flag |= tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT") == 'y';
> +		flag = tst_kconfig_get(KERNEL_CONFIG1) == 'y';
> +		flag |= tst_kconfig_get(KERNEL_CONFIG2) == 'y';

I don't think that defining two constant macros for the kernel config 
options makes sense here. It's just a coincidence that both PPC and x86 
currently have two possible config keys. When we add special support for 
a third arch that has only one (or e.g. a third option for x86), this 
code will need to be reworked back to what was in v1. Or you could check 
all 4 option regardless of arch, the worst thing that'll happen is that 
the test setup will be slightly slower.

> +
>   		if (flag && tst_secureboot_enabled() > 0)
>   			return 1;
>   

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