On Mon, 22 Jun 2026 10:00:07 -0500 Ian Bridges <[email protected]> wrote:
> In preparation for removing the deprecated strlcat() API[1], replace the > strscpy()/strlcat() chain in selinux_ima_collect_state() with a struct > seq_buf, which tracks the write position and remaining space internally. > > The seven open-coded WARN_ON(rc >= buf_len) truncation checks become a > single seq_buf_has_overflowed() check after the string is built. The > kzalloc() and its exact-size computation are unchanged, so the > measurement string passed to IMA is unchanged. > > Link: https://github.com/KSPP/linux/issues/370 [1] > Signed-off-by: Ian Bridges <[email protected]> > --- > security/selinux/ima.c | 35 ++++++++++++++--------------------- > 1 file changed, 14 insertions(+), 21 deletions(-) > > diff --git a/security/selinux/ima.c b/security/selinux/ima.c > index aa34da9b0aeb..3d81093d16aa 100644 > --- a/security/selinux/ima.c > +++ b/security/selinux/ima.c > @@ -9,6 +9,7 @@ > */ > #include <linux/vmalloc.h> > #include <linux/ima.h> > +#include <linux/seq_buf.h> > #include "security.h" > #include "ima.h" > > @@ -21,8 +22,9 @@ > static char *selinux_ima_collect_state(void) > { > const char *on = "=1;", *off = "=0;"; > + struct seq_buf s; > char *buf; > - int buf_len, len, i, rc; > + int buf_len, len, i; > > buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1; > > @@ -34,33 +36,24 @@ static char *selinux_ima_collect_state(void) > if (!buf) > return NULL; > > - rc = strscpy(buf, "initialized", buf_len); > - WARN_ON(rc < 0); > + seq_buf_init(&s, buf, buf_len); That is silly, you need the length of the buffer not the length of a string that is the expected length of the output. > > - rc = strlcat(buf, selinux_initialized() ? on : off, buf_len); > - WARN_ON(rc >= buf_len); > + seq_buf_puts(&s, "initialized"); > + seq_buf_puts(&s, selinux_initialized() ? on : off); > > - rc = strlcat(buf, "enforcing", buf_len); > - WARN_ON(rc >= buf_len); > + seq_buf_puts(&s, "enforcing"); > + seq_buf_puts(&s, enforcing_enabled() ? on : off); > > - rc = strlcat(buf, enforcing_enabled() ? on : off, buf_len); > - WARN_ON(rc >= buf_len); > - > - rc = strlcat(buf, "checkreqprot", buf_len); > - WARN_ON(rc >= buf_len); > - > - rc = strlcat(buf, checkreqprot_get() ? on : off, buf_len); > - WARN_ON(rc >= buf_len); > + seq_buf_puts(&s, "checkreqprot"); > + seq_buf_puts(&s, checkreqprot_get() ? on : off); That lot would be easier to read as a seq_printf() - with %d and kill 'on' and 'off'. Why does 'security' code so often look like c**p. David > > for (i = 0; i < __POLICYDB_CAP_MAX; i++) { > - rc = strlcat(buf, selinux_policycap_names[i], buf_len); > - WARN_ON(rc >= buf_len); > - > - rc = strlcat(buf, selinux_state.policycap[i] ? on : off, > - buf_len); > - WARN_ON(rc >= buf_len); > + seq_buf_puts(&s, selinux_policycap_names[i]); > + seq_buf_puts(&s, selinux_state.policycap[i] ? on : off); > } > > + WARN_ON(seq_buf_has_overflowed(&s)); > + > return buf; > } >
