On 6/22/2026 9:53 AM, David Laight wrote: > 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.
Sturgeon's Law. Also, it's pretty rare that developers outside the security community look at the security code. It usually only happens when there's a global change, like this one. And to be clear, reviews like this *are* appreciated.
