[PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[Ian Bridges]

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.

Each field is written with seq_buf_printf() using a "=%d;" format, which
removes the open-coded "=1;"/"=0;" constants. The seven per-append
WARN_ON(rc >= buf_len) truncation checks are replaced by a single
seq_buf_has_overflowed() check after the string is built.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
Changed in v2: replace the v1 seq_buf_puts() pairs with seq_buf_printf()
using a "=%d;" format, which drops the open-coded "=1;"/"=0;" constants.

v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/

I didn't change the precomputation of the string size. An alternative,
which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
mm/memcontrol.c), is to drop the precomputation and allocate an oversized
fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
happy to rework the patch to adopt that alternative.

 security/selinux/ima.c | 40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/security/selinux/ima.c b/security/selinux/ima.c
index aa34da9b0aeb..cb0efa2fc1ad 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"
 
@@ -20,46 +21,31 @@
  */
 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, suffix_len, i;
 
 	buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
+	suffix_len = strlen("=0;");
 
-	len = strlen(on);
 	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
-		buf_len += strlen(selinux_policycap_names[i]) + len;
+		buf_len += strlen(selinux_policycap_names[i]) + suffix_len;
 
 	buf = kzalloc(buf_len, GFP_KERNEL);
 	if (!buf)
 		return NULL;
 
-	rc = strscpy(buf, "initialized", buf_len);
-	WARN_ON(rc < 0);
+	seq_buf_init(&s, buf, buf_len);
 
-	rc = strlcat(buf, selinux_initialized() ? on : off, buf_len);
-	WARN_ON(rc >= buf_len);
+	seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;",
+		       selinux_initialized(), enforcing_enabled(),
+		       checkreqprot_get());
 
-	rc = strlcat(buf, "enforcing", buf_len);
-	WARN_ON(rc >= buf_len);
-
-	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);
-
-	for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
-		rc = strlcat(buf, selinux_policycap_names[i], buf_len);
-		WARN_ON(rc >= buf_len);
+	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
+		seq_buf_printf(&s, "%s=%d;", selinux_policycap_names[i],
+			       selinux_state.policycap[i]);
 
-		rc = strlcat(buf, selinux_state.policycap[i] ? on : off,
-			buf_len);
-		WARN_ON(rc >= buf_len);
-	}
+	WARN_ON(seq_buf_has_overflowed(&s));
 
 	return buf;
 }
-- 
2.47.3

Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[Stephen Smalley]

On Fri, Jun 26, 2026 at 11:57 AM Ian Bridges <icb@fastmail.org> 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.
>
> Each field is written with seq_buf_printf() using a "=%d;" format, which
> removes the open-coded "=1;"/"=0;" constants. The seven per-append
> WARN_ON(rc >= buf_len) truncation checks are replaced by a single
> seq_buf_has_overflowed() check after the string is built.
>
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Ian Bridges <icb@fastmail.org>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
> Changed in v2: replace the v1 seq_buf_puts() pairs with seq_buf_printf()
> using a "=%d;" format, which drops the open-coded "=1;"/"=0;" constants.
>
> v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/
>
> I didn't change the precomputation of the string size. An alternative,
> which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
> mm/memcontrol.c), is to drop the precomputation and allocate an oversized
> fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
> happy to rework the patch to adopt that alternative.
>
>  security/selinux/ima.c | 40 +++++++++++++---------------------------
>  1 file changed, 13 insertions(+), 27 deletions(-)
>
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> index aa34da9b0aeb..cb0efa2fc1ad 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"
>
> @@ -20,46 +21,31 @@
>   */
>  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, suffix_len, i;
>
>         buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
> +       suffix_len = strlen("=0;");
>
> -       len = strlen(on);
>         for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> -               buf_len += strlen(selinux_policycap_names[i]) + len;
> +               buf_len += strlen(selinux_policycap_names[i]) + suffix_len;
>
>         buf = kzalloc(buf_len, GFP_KERNEL);
>         if (!buf)
>                 return NULL;
>
> -       rc = strscpy(buf, "initialized", buf_len);
> -       WARN_ON(rc < 0);
> +       seq_buf_init(&s, buf, buf_len);
>
> -       rc = strlcat(buf, selinux_initialized() ? on : off, buf_len);
> -       WARN_ON(rc >= buf_len);
> +       seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;",
> +                      selinux_initialized(), enforcing_enabled(),
> +                      checkreqprot_get());
>
> -       rc = strlcat(buf, "enforcing", buf_len);
> -       WARN_ON(rc >= buf_len);
> -
> -       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);
> -
> -       for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
> -               rc = strlcat(buf, selinux_policycap_names[i], buf_len);
> -               WARN_ON(rc >= buf_len);
> +       for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> +               seq_buf_printf(&s, "%s=%d;", selinux_policycap_names[i],
> +                              selinux_state.policycap[i]);
>
> -               rc = strlcat(buf, selinux_state.policycap[i] ? on : off,
> -                       buf_len);
> -               WARN_ON(rc >= buf_len);
> -       }
> +       WARN_ON(seq_buf_has_overflowed(&s));
>
>         return buf;
>  }
> --
> 2.47.3
>

Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[David Laight]

On Fri, 26 Jun 2026 10:57:10 -0500
Ian Bridges <icb@fastmail.org> 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.
> 
> Each field is written with seq_buf_printf() using a "=%d;" format, which
> removes the open-coded "=1;"/"=0;" constants. The seven per-append
> WARN_ON(rc >= buf_len) truncation checks are replaced by a single
> seq_buf_has_overflowed() check after the string is built.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Ian Bridges <icb@fastmail.org>
> ---
> Changed in v2: replace the v1 seq_buf_puts() pairs with seq_buf_printf()
> using a "=%d;" format, which drops the open-coded "=1;"/"=0;" constants.

Did you verify that all the values are 0/1 otherwise a !! prefix might
be needed.

> 
> v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/
> 
> I didn't change the precomputation of the string size. An alternative,
> which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
> mm/memcontrol.c), is to drop the precomputation and allocate an oversized
> fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
> happy to rework the patch to adopt that alternative.

That would be reasonable, the output is always exactly the same length and
the buffer is freed just after being allocated.
A comment that there are 3 + 15 strings and all are under 32 bytes so 1k
is plenty would suffice.

	David

> 
>  security/selinux/ima.c | 40 +++++++++++++---------------------------
>  1 file changed, 13 insertions(+), 27 deletions(-)
> 
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> index aa34da9b0aeb..cb0efa2fc1ad 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"
>  
> @@ -20,46 +21,31 @@
>   */
>  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, suffix_len, i;
>  
>  	buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
> +	suffix_len = strlen("=0;");
>  
> -	len = strlen(on);
>  	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> -		buf_len += strlen(selinux_policycap_names[i]) + len;
> +		buf_len += strlen(selinux_policycap_names[i]) + suffix_len;
>  
>  	buf = kzalloc(buf_len, GFP_KERNEL);
>  	if (!buf)
>  		return NULL;
>  
> -	rc = strscpy(buf, "initialized", buf_len);
> -	WARN_ON(rc < 0);
> +	seq_buf_init(&s, buf, buf_len);
>  
> -	rc = strlcat(buf, selinux_initialized() ? on : off, buf_len);
> -	WARN_ON(rc >= buf_len);
> +	seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;",
> +		       selinux_initialized(), enforcing_enabled(),
> +		       checkreqprot_get());
>  
> -	rc = strlcat(buf, "enforcing", buf_len);
> -	WARN_ON(rc >= buf_len);
> -
> -	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);
> -
> -	for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
> -		rc = strlcat(buf, selinux_policycap_names[i], buf_len);
> -		WARN_ON(rc >= buf_len);
> +	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> +		seq_buf_printf(&s, "%s=%d;", selinux_policycap_names[i],
> +			       selinux_state.policycap[i]);
>  
> -		rc = strlcat(buf, selinux_state.policycap[i] ? on : off,
> -			buf_len);
> -		WARN_ON(rc >= buf_len);
> -	}
> +	WARN_ON(seq_buf_has_overflowed(&s));
>  
>  	return buf;
>  }

Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[Ian Bridges]

On Sat, Jun 27, 2026 at 10:49:14PM +0100, David Laight wrote:
> On Fri, 26 Jun 2026 10:57:10 -0500
> Ian Bridges <icb@fastmail.org> 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.
> > 
> > Each field is written with seq_buf_printf() using a "=%d;" format, which
> > removes the open-coded "=1;"/"=0;" constants. The seven per-append
> > WARN_ON(rc >= buf_len) truncation checks are replaced by a single
> > seq_buf_has_overflowed() check after the string is built.
> > 
> > Link: https://github.com/KSPP/linux/issues/370 [1]
> > Signed-off-by: Ian Bridges <icb@fastmail.org>
> > ---
> > Changed in v2: replace the v1 seq_buf_puts() pairs with seq_buf_printf()
> > using a "=%d;" format, which drops the open-coded "=1;"/"=0;" constants.
> 
> Did you verify that all the values are 0/1 otherwise a !! prefix might
> be needed.

Yes. selinux_initialized(), enforcing_enabled() and checkreqprot_get()
all return bool, and selinux_state.policycap[] is a bool array.
checkreqprot_get() always returns 0. The others read bool fields that
are written only by normal assignment (WRITE_ONCE() or
smp_store_release()), which normalizes the stored value to 0 or 1. So
"%d" prints "0" or "1", the same as the old ? "=1;" : "=0;" mapping, and
no !! is needed.

> 
> > 
> > v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/
> > 
> > I didn't change the precomputation of the string size. An alternative,
> > which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
> > mm/memcontrol.c), is to drop the precomputation and allocate an oversized
> > fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
> > happy to rework the patch to adopt that alternative.
> 
> That would be reasonable, the output is always exactly the same length and
> the buffer is freed just after being allocated.
> A comment that there are 3 + 15 strings and all are under 32 bytes so 1k
> is plenty would suffice.
> 
> 	David
>

I didn't originally take this route because a fixed buffer bakes in an
invariant (the whole string must stay under 1K) with nothing to enforce
it at compile time. If more capabilities are added later, or a name grows
longer, the string could overflow. That shows up only at runtime, as the
seq_buf_has_overflowed() WARN, and the measured selinux-state string is
silently truncated.

Thanks for the review.

Ian

> > 
> >  security/selinux/ima.c | 40 +++++++++++++---------------------------
> >  1 file changed, 13 insertions(+), 27 deletions(-)
> > 
> > diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> > index aa34da9b0aeb..cb0efa2fc1ad 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"
> >  
> > @@ -20,46 +21,31 @@
> >   */
> >  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, suffix_len, i;
> >  
> >  	buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
> > +	suffix_len = strlen("=0;");
> >  
> > -	len = strlen(on);
> >  	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> > -		buf_len += strlen(selinux_policycap_names[i]) + len;
> > +		buf_len += strlen(selinux_policycap_names[i]) + suffix_len;
> >  
> >  	buf = kzalloc(buf_len, GFP_KERNEL);
> >  	if (!buf)
> >  		return NULL;
> >  
> > -	rc = strscpy(buf, "initialized", buf_len);
> > -	WARN_ON(rc < 0);
> > +	seq_buf_init(&s, buf, buf_len);
> >  
> > -	rc = strlcat(buf, selinux_initialized() ? on : off, buf_len);
> > -	WARN_ON(rc >= buf_len);
> > +	seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;",
> > +		       selinux_initialized(), enforcing_enabled(),
> > +		       checkreqprot_get());
> >  
> > -	rc = strlcat(buf, "enforcing", buf_len);
> > -	WARN_ON(rc >= buf_len);
> > -
> > -	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);
> > -
> > -	for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
> > -		rc = strlcat(buf, selinux_policycap_names[i], buf_len);
> > -		WARN_ON(rc >= buf_len);
> > +	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> > +		seq_buf_printf(&s, "%s=%d;", selinux_policycap_names[i],
> > +			       selinux_state.policycap[i]);
> >  
> > -		rc = strlcat(buf, selinux_state.policycap[i] ? on : off,
> > -			buf_len);
> > -		WARN_ON(rc >= buf_len);
> > -	}
> > +	WARN_ON(seq_buf_has_overflowed(&s));
> >  
> >  	return buf;
> >  }
> 

Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[David Laight]

On Wed, 1 Jul 2026 00:54:11 -0500
Ian Bridges <icb@fastmail.org> wrote:

> On Sat, Jun 27, 2026 at 10:49:14PM +0100, David Laight wrote:
> > On Fri, 26 Jun 2026 10:57:10 -0500
> > Ian Bridges <icb@fastmail.org> wrote:
...
> > > 
> > > v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/
> > > 
> > > I didn't change the precomputation of the string size. An alternative,
> > > which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
> > > mm/memcontrol.c), is to drop the precomputation and allocate an oversized
> > > fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
> > > happy to rework the patch to adopt that alternative.  
> > 
> > That would be reasonable, the output is always exactly the same length and
> > the buffer is freed just after being allocated.
> > A comment that there are 3 + 15 strings and all are under 32 bytes so 1k
> > is plenty would suffice.
> > 
> > 	David
> >  
> 
> I didn't originally take this route because a fixed buffer bakes in an
> invariant (the whole string must stay under 1K) with nothing to enforce
> it at compile time. If more capabilities are added later, or a name grows
> longer, the string could overflow. That shows up only at runtime, as the
> seq_buf_has_overflowed() WARN, and the measured selinux-state string is
> silently truncated.

You could use '32 * (3 + NUM_CAPAPILITIES)'.
IIRC the longest is 24 characters (plus 2 for the =[01]) so that will be plenty.

	David


> 
> Thanks for the review.
> 
> Ian
> 
> > > 
> > >  security/selinux/ima.c | 40 +++++++++++++---------------------------
> > >  1 file changed, 13 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> > > index aa34da9b0aeb..cb0efa2fc1ad 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"
> > >  
> > > @@ -20,46 +21,31 @@
> > >   */
> > >  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, suffix_len, i;
> > >  
> > >  	buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
> > > +	suffix_len = strlen("=0;");
> > >  
> > > -	len = strlen(on);
> > >  	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> > > -		buf_len += strlen(selinux_policycap_names[i]) + len;
> > > +		buf_len += strlen(selinux_policycap_names[i]) + suffix_len;
> > >  
> > >  	buf = kzalloc(buf_len, GFP_KERNEL);
> > >  	if (!buf)
> > >  		return NULL;
> > >  
> > > -	rc = strscpy(buf, "initialized", buf_len);
> > > -	WARN_ON(rc < 0);
> > > +	seq_buf_init(&s, buf, buf_len);
> > >  
> > > -	rc = strlcat(buf, selinux_initialized() ? on : off, buf_len);
> > > -	WARN_ON(rc >= buf_len);
> > > +	seq_buf_printf(&s, "initialized=%d;enforcing=%d;checkreqprot=%d;",
> > > +		       selinux_initialized(), enforcing_enabled(),
> > > +		       checkreqprot_get());
> > >  
> > > -	rc = strlcat(buf, "enforcing", buf_len);
> > > -	WARN_ON(rc >= buf_len);
> > > -
> > > -	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);
> > > -
> > > -	for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
> > > -		rc = strlcat(buf, selinux_policycap_names[i], buf_len);
> > > -		WARN_ON(rc >= buf_len);
> > > +	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
> > > +		seq_buf_printf(&s, "%s=%d;", selinux_policycap_names[i],
> > > +			       selinux_state.policycap[i]);
> > >  
> > > -		rc = strlcat(buf, selinux_state.policycap[i] ? on : off,
> > > -			buf_len);
> > > -		WARN_ON(rc >= buf_len);
> > > -	}
> > > +	WARN_ON(seq_buf_has_overflowed(&s));
> > >  
> > >  	return buf;
> > >  }  
> >   

Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state()

[Paul Moore]

On Jun 26, 2026 Ian Bridges <icb@fastmail.org> 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.
> 
> Each field is written with seq_buf_printf() using a "=%d;" format, which
> removes the open-coded "=1;"/"=0;" constants. The seven per-append
> WARN_ON(rc >= buf_len) truncation checks are replaced by a single
> seq_buf_has_overflowed() check after the string is built.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Ian Bridges <icb@fastmail.org>
> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> Changed in v2: replace the v1 seq_buf_puts() pairs with seq_buf_printf()
> using a "=%d;" format, which drops the open-coded "=1;"/"=0;" constants.
> 
> v1: https://lore.kernel.org/all/ajlN94VO7BYNUTAy@dev/
> 
> I didn't change the precomputation of the string size. An alternative,
> which is used by other seq_buf callers (e.g. kernel/rcu/refscale.c,
> mm/memcontrol.c), is to drop the precomputation and allocate an oversized
> fixed buffer, relying on the seq_buf overflow check as a backstop. I'm
> happy to rework the patch to adopt that alternative.

Another option would be to split out the length calculation and only do
it once at boot, e.g. call a selinux_ima_calculate_len() function from
selinux_init(), and then reuse that length in
selinux_ima_collect_state().

What you're doing here is no worse than what we currently have so I'm
going to go ahead and merge it into selinux/dev, but if you could split
out the length calculation I think that would be worthwhile.  Thanks!

>  security/selinux/ima.c | 40 +++++++++++++---------------------------
>  1 file changed, 13 insertions(+), 27 deletions(-)

--
paul-moore.com