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

[R Nageswara Sastry]

On 05/09/23 2:53 pm, Martin Doucha wrote:
> 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.


Sure, then I will fall back to the v1 approach for these checking kernel 
config code.
ie.
+		#if defined(__powerpc64__) || defined(__ppc64__)
+		flag = tst_kconfig_get("CONFIG_SECURITY_LOCKDOWN_LSM") == 'y';
+		flag |= tst_kconfig_get("CONFIG_SECURITY_LOCKDOWN_LSM_EARLY") == 'y';
+		#else
  		flag = tst_kconfig_get("CONFIG_EFI_SECURE_BOOT_LOCK_DOWN") == 'y';
  		flag |= tst_kconfig_get("CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT") == 'y';
+		#endif


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

-- 
Thanks and Regards
R.Nageswara Sastry