Re: [PATCH] selinux: compute the IMA configuration settings string length once at boot
Stephen Smalley <[email protected]>
| Newsgroups | org.kernel.vger.selinux,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAEjxPJ4h=O8NbkGLbc8YZEJSVpV=1-gmROG-imfzDVtru4qVNA@mail.gmail.com> |
On Tue, Jul 14, 2026 at 12:09 AM Ian Bridges <[email protected]> wrote: > > selinux_ima_collect_state() builds a string of the current SELinux > configuration settings. The string lists each setting as a name and > one digit. The length of the string therefore never changes, but is > still recomputed on every call. > > Add selinux_ima_config_len_init() to compute the length once during > selinux_init(). Update selinux_ima_collect_state() to use the stored > length. > > Suggested-by: Paul Moore <[email protected]> > Link: https://lore.kernel.org/r/[email protected] > Signed-off-by: Ian Bridges <[email protected]> Reviewed-by: Stephen Smalley <[email protected]> > --- > The produced string is unchanged. selinux_ima_collect_state() now > depends on selinux_ima_config_len_init() having run. The call sits in > selinux_init(), as suggested in the review of the seq_buf conversion. > > The length computation itself moves verbatim. The strlen() + 1 on > the string literal could become sizeof(), which counts the > terminator. That rewrite was left out so the moved block stays > identical to the applied code. > > The patch was tested as follows. > > - W=1 build of security/selinux with CONFIG_IMA=y, zero warnings. > - A userspace differential harness compiled the old and the new > functions side by side, covering every combination of the settings > and the policy capabilities. The outputs were byte identical. The > stored length was an exact fit for the produced string in every > case, with no overflow and no slack. > > security/selinux/hooks.c | 3 +++ > security/selinux/ima.c | 33 ++++++++++++++++++++++++--------- > security/selinux/include/ima.h | 4 ++++ > 3 files changed, 31 insertions(+), 9 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index f4a4edbff2bd..1ead2eee1944 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -107,6 +107,7 @@ > #include "netlabel.h" > #include "audit.h" > #include "avc_ss.h" > +#include "ima.h" > > #define SELINUX_INODE_INIT_XATTRS 1 > > @@ -7856,6 +7857,8 @@ static __init int selinux_init(void) > > hashtab_cache_init(); > > + selinux_ima_config_len_init(); > + > security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), > &selinux_lsmid); > > diff --git a/security/selinux/ima.c b/security/selinux/ima.c > index cb0efa2fc1ad..cda68122e032 100644 > --- a/security/selinux/ima.c > +++ b/security/selinux/ima.c > @@ -13,6 +13,27 @@ > #include "security.h" > #include "ima.h" > > +static int selinux_ima_config_len __ro_after_init; > + > +/* > + * selinux_ima_config_len_init - Compute the configuration settings string length > + * > + * The string is fixed text plus one digit per setting, so its length > + * is known at boot. > + */ > +void __init selinux_ima_config_len_init(void) > +{ > + int buf_len, suffix_len, i; > + > + buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1; > + suffix_len = strlen("=0;"); > + > + for (i = 0; i < __POLICYDB_CAP_MAX; i++) > + buf_len += strlen(selinux_policycap_names[i]) + suffix_len; > + > + selinux_ima_config_len = buf_len; > +} > + > /* > * selinux_ima_collect_state - Read selinux configuration settings > * > @@ -23,19 +44,13 @@ static char *selinux_ima_collect_state(void) > { > struct seq_buf s; > char *buf; > - int buf_len, suffix_len, i; > + int i; > > - buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1; > - suffix_len = strlen("=0;"); > - > - for (i = 0; i < __POLICYDB_CAP_MAX; i++) > - buf_len += strlen(selinux_policycap_names[i]) + suffix_len; > - > - buf = kzalloc(buf_len, GFP_KERNEL); > + buf = kzalloc(selinux_ima_config_len, GFP_KERNEL); > if (!buf) > return NULL; > > - seq_buf_init(&s, buf, buf_len); > + seq_buf_init(&s, buf, selinux_ima_config_len); > > seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;", > selinux_initialized(), enforcing_enabled(), > diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h > index 38ab302f5946..d7d18f030d3d 100644 > --- a/security/selinux/include/ima.h > +++ b/security/selinux/include/ima.h > @@ -14,9 +14,13 @@ > #include "security.h" > > #ifdef CONFIG_IMA > +void __init selinux_ima_config_len_init(void); > extern void selinux_ima_measure_state(void); > extern void selinux_ima_measure_state_locked(void); > #else > +static inline void selinux_ima_config_len_init(void) > +{ > +} > static inline void selinux_ima_measure_state(void) > { > } > -- > 2.47.3 >