| Message ID | 20220606101042.89638-1-xiujianfeng@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] evm: Use IS_ENABLED to initialize .enabled | expand |
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 > }, > };
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 >> }, >> }; >
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
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
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
在 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 > > .
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
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 }, };
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(-)