evm: Use IS_ENABLED to initialize .enabled

[PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Xiu Jianfeng 3 years, 10 months ago

Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
initialize .enabled, minor simplicity improvement.

Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
---
 security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index cc88f02c7562..397fea5b3fa6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -36,42 +36,36 @@ static const char * const integrity_status_msg[] = {
 int evm_hmac_attrs;
 
 static struct xattr_list evm_config_default_xattrnames[] = {
-	{.name = XATTR_NAME_SELINUX,
-#ifdef CONFIG_SECURITY_SELINUX
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SELINUX,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
 	},
-	{.name = XATTR_NAME_SMACK,
-#ifdef CONFIG_SECURITY_SMACK
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACK,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
 	},
-	{.name = XATTR_NAME_SMACKEXEC,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKEXEC,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_SMACKTRANSMUTE,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKTRANSMUTE,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_SMACKMMAP,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKMMAP,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_APPARMOR,
-#ifdef CONFIG_SECURITY_APPARMOR
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_APPARMOR,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
 	},
-	{.name = XATTR_NAME_IMA,
-#ifdef CONFIG_IMA_APPRAISE
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_IMA,
+	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
 	},
-	{.name = XATTR_NAME_CAPS,
+	{
+	 .name = XATTR_NAME_CAPS,
 	 .enabled = true
 	},
 };
-- 
2.17.1

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Mimi Zohar 3 years, 9 months ago

On Mon, 2022-06-06 at 18:10 +0800, Xiu Jianfeng wrote:
> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> initialize .enabled, minor simplicity improvement.
> 
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>

Thanks, Xiu.   This patch is now queued in next-testing.

Mimi

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Ahmad Fatoum 3 years, 10 months ago

On 06.06.22 12:10, Xiu Jianfeng wrote:
> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> initialize .enabled, minor simplicity improvement.
> 
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 29 deletions(-)
> 
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cc88f02c7562..397fea5b3fa6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -36,42 +36,36 @@ static const char * const integrity_status_msg[] = {
>  int evm_hmac_attrs;
>  
>  static struct xattr_list evm_config_default_xattrnames[] = {
> -	{.name = XATTR_NAME_SELINUX,
> -#ifdef CONFIG_SECURITY_SELINUX
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SELINUX,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
>  	},
> -	{.name = XATTR_NAME_SMACK,
> -#ifdef CONFIG_SECURITY_SMACK
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACK,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
>  	},
> -	{.name = XATTR_NAME_SMACKEXEC,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKEXEC,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_SMACKTRANSMUTE,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKTRANSMUTE,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_SMACKMMAP,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKMMAP,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_APPARMOR,
> -#ifdef CONFIG_SECURITY_APPARMOR
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_APPARMOR,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
>  	},
> -	{.name = XATTR_NAME_IMA,
> -#ifdef CONFIG_IMA_APPRAISE
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_IMA,
> +	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
>  	},
> -	{.name = XATTR_NAME_CAPS,
> +	{
> +	 .name = XATTR_NAME_CAPS,
>  	 .enabled = true
>  	},
>  };


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by xiujianfeng 3 years, 10 months ago

Hi, Ahmad

在 2022/6/7 14:06, Ahmad Fatoum 写道:
> On 06.06.22 12:10, Xiu Jianfeng wrote:
>> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
>> initialize .enabled, minor simplicity improvement.
>>
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
thank you for the review, and I'm not sure if this patch has been 
picked, so frendly ping here...
>> ---
>>   security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
>>   1 file changed, 23 insertions(+), 29 deletions(-)
>>
>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> index cc88f02c7562..397fea5b3fa6 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -36,42 +36,36 @@ static const char * const integrity_status_msg[] = {
>>   int evm_hmac_attrs;
>>   
>>   static struct xattr_list evm_config_default_xattrnames[] = {
>> -	{.name = XATTR_NAME_SELINUX,
>> -#ifdef CONFIG_SECURITY_SELINUX
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SELINUX,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
>>   	},
>> -	{.name = XATTR_NAME_SMACK,
>> -#ifdef CONFIG_SECURITY_SMACK
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACK,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
>>   	},
>> -	{.name = XATTR_NAME_SMACKEXEC,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKEXEC,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_SMACKTRANSMUTE,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKTRANSMUTE,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_SMACKMMAP,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKMMAP,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_APPARMOR,
>> -#ifdef CONFIG_SECURITY_APPARMOR
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_APPARMOR,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
>>   	},
>> -	{.name = XATTR_NAME_IMA,
>> -#ifdef CONFIG_IMA_APPRAISE
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_IMA,
>> +	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
>>   	},
>> -	{.name = XATTR_NAME_CAPS,
>> +	{
>> +	 .name = XATTR_NAME_CAPS,
>>   	 .enabled = true
>>   	},
>>   };
>

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Mimi Zohar 3 years, 10 months ago

On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> Hi, Ahmad
> 
> 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > On 06.06.22 12:10, Xiu Jianfeng wrote:
> >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> >> initialize .enabled, minor simplicity improvement.

The difference between using ifdef's and IS_ENABLED is when the
decision is made - build time, run time.   Please update the patch
description providing an explanation for needing to make the decision
at run time.

thanks,

Mimi

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Eric Biggers 3 years, 10 months ago

On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
> On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> > Hi, Ahmad
> > 
> > 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > > On 06.06.22 12:10, Xiu Jianfeng wrote:
> > >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> > >> initialize .enabled, minor simplicity improvement.
> 
> The difference between using ifdef's and IS_ENABLED is when the
> decision is made - build time, run time.   Please update the patch
> description providing an explanation for needing to make the decision
> at run time.
> 
> thanks,

IS_ENABLED() is a compile time constant.  So the patch looks fine to me.

- Eric

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by Mimi Zohar 3 years, 10 months ago

On Tue, 2022-06-21 at 19:17 -0700, Eric Biggers wrote:
> On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
> > On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> > > Hi, Ahmad
> > > 
> > > 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > > > On 06.06.22 12:10, Xiu Jianfeng wrote:
> > > >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> > > >> initialize .enabled, minor simplicity improvement.
> > 
> > The difference between using ifdef's and IS_ENABLED is when the
> > decision is made - build time, run time.   Please update the patch
> > description providing an explanation for needing to make the decision
> > at run time.
> > 
> > thanks,
> 
> IS_ENABLED() is a compile time constant.  So the patch looks fine to me.

Thanks, Eric, for the clarification.

As LSMs are only builtin, why the need for using IS_ENABLED as opposed
to IS_BUILTIN?

#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))

thanks,

Mimi

Re: [PATCH -next] evm: Use IS_ENABLED to initialize .enabled

Posted by xiujianfeng 3 years, 10 months ago

在 2022/6/27 0:13, Mimi Zohar 写道:
> On Tue, 2022-06-21 at 19:17 -0700, Eric Biggers wrote:
>> On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
>>> On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
>>>> Hi, Ahmad
>>>>
>>>> 在 2022/6/7 14:06, Ahmad Fatoum 写道:
>>>>> On 06.06.22 12:10, Xiu Jianfeng wrote:
>>>>>> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
>>>>>> initialize .enabled, minor simplicity improvement.
>>> The difference between using ifdef's and IS_ENABLED is when the
>>> decision is made - build time, run time.   Please update the patch
>>> description providing an explanation for needing to make the decision
>>> at run time.
>>>
>>> thanks,
>> IS_ENABLED() is a compile time constant.  So the patch looks fine to me.
> Thanks, Eric, for the clarification.
>
> As LSMs are only builtin, why the need for using IS_ENABLED as opposed
> to IS_BUILTIN?
>
> #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>
> thanks,

I think IS_ENALBED() is a bit more generic, maybe. here is another 
example in rcutorture.c

which uses IS_ENABLED() to initialize the member in structure:

static struct rcu_torture_ops rcu_ops = {

...

.can_boost = IS_ENABLED(CONFIG_RCU_BOOST),

...

};

Do you want me to change IS_ENABLED() to IS_BUILTIN()?

>
> Mimi
>
> .