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

[Paul Moore]

On Jun 26, 2026 Ian Bridges <icb@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Reviewed-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
> ---
> 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