mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Ian Bridges <icb@fastmail.org>,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	Ondrej Mosnacek <omosnace@redhat.com>,
	selinux@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2] selinux: replace strlcat() with seq_buf in  selinux_ima_collect_state()
Date: Mon, 13 Jul 2026 18:15:28 -0400	[thread overview]
Message-ID: <df755e0282dab3b932d19aceab71b7d7@paul-moore.com> (raw)
In-Reply-To: <aj6hVjECikvYtnED@dev>

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

      parent reply	other threads:[~2026-07-13 22:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 15:57 Ian Bridges
2026-06-26 18:52 ` Stephen Smalley
2026-06-27 21:49 ` David Laight
2026-07-01  5:54   ` Ian Bridges
2026-07-01  9:52     ` David Laight
2026-07-13 22:15 ` Paul Moore [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=df755e0282dab3b932d19aceab71b7d7@paul-moore.com \
    --to=paul@paul-moore.com \
    --cc=icb@fastmail.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=omosnace@redhat.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®