From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2FB973EA96E for ; Wed, 1 Jul 2026 09:52:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782899562; cv=none; b=OjkF2Y/D0uKPo/9cA2Lqsf5JwFBdd7qhOM8VpLDUZABVJTbgOPheQmyH3h1WkkWB6ZMx81LFgHBQcA1eneU3ndSb/XNvfgDah9qftzBMEHP7+WFmtN/h7sPK79k7zF/l5UBSJ8opSfZOWUJCLxE1Cx2ljXK8pGBv0ENSbDH2Zj0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782899562; c=relaxed/simple; bh=ceEp1yWHS2rI9FV3hh8oAtV9bvoPQs9rQQCGvhLiECc=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=UWnCcEPNbc6hdQM2oAl9YjWOA+h2MvgoFtszmakzul6Rib7e5VCWoUYUt7pWnQk4vyoGBUTg+8Odgo5L2DAOIAFp6PQ/x0AIZE3JBb6KM/Q1vcbrGR28j9frHbEdoaKVSoGDshJP8eUdB7wU3r+mB5na5VV3/Qccrd+L2WCPWQ0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=r/MLZ0+e; arc=none smtp.client-ip=209.85.128.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="r/MLZ0+e" Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-493a97fad2fso3996895e9.0 for ; Wed, 01 Jul 2026 02:52:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782899560; x=1783504360; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=Oqbno7ujq0aZ6PN45IXpH4UapLNFI2wSOuPDyXwmuAc=; b=r/MLZ0+ePDGwm/FSe1ujm8J0+6z1fuALddOKz1fQ6BL7L3bt+c9SmIH91jqz/orENZ ziByY8TNS6ZQgJ8SBkyPSBWODj3fTxg5Mg7eG1vcky64KcYitvhAkl6ULCd3oSZVAEYq U1LhL4c+5TllnXq27K6LhVLH69iPG1rfCmxb5MoHUIirEpTI6dS/WD5FzU/ZduRUUV/E qgaF6kUzObkreHpncFp/0yem5GrMhwq5m91NQbzPOmZxFtZv1EmZckWVwCqf9bm3ffSd 3xqMc3zoq2pWf+4mQ0UEHdJ1Tkxabipyc9wapq1GIky51FsMlv4SJiSbqGrop82TIqg5 N3Hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782899560; x=1783504360; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=Oqbno7ujq0aZ6PN45IXpH4UapLNFI2wSOuPDyXwmuAc=; b=HUDKO70wdKCbFHiD742H8m8sZdbJAUQFZbdQ+U/r0K4tBfb9ei0i5qKrNMnQ/KG+DM MnzlXBzO5pXWs5Wd1YW3lcpfoHUUEl48Jw0sfoNjRgbigUZxDS0SNwPdUWGkv+4U7H9k J1ay1cU2uyolDORjjh7e7+E4HdBapF5EZXYWQlNbbYHvOFcrNMME0HVf7RhG/tK7zWKc XphJ3zylQBrXrJ6ZuGhc059JMIhasKglyh820VEP+pGOlFH8F+muL4Msn5SmLnG1lbWZ WCHCNeAQ41I7lmlz0OOERUEi+pp3pXIpPyKn4DxtkJs1yjkOwgTmKfxuO7Xqa+jwdXzG kHFg== X-Forwarded-Encrypted: i=1; AFNElJ++SdisQo3qbf9P0xsdwMSic9EDQakAFJYeIjcBb3rKRqzqhBWoU2/ord1YR8MgGLBW7zASynpOuuXsQx8=@vger.kernel.org X-Gm-Message-State: AOJu0YxCLo8x1iB7FHcA1iyWA6SK7gPmsSBeEiFgT6QBoYIiUEBGwYVa GrYGC1Ilp6kewO+sxcwzi9ZSJxqJOkOL0tBTqI+XNT44WFwTkkfGN9xQ X-Gm-Gg: AfdE7clgPWM7h073UeAd0BrnPBwgczKVBrueWe+r66dt4YKGOPBFsoq4GMTXhDuaNsm GFd+30qGs33UIO/I9T3/q5L6fBslrEXYOHoU4nAOfF63j1DIBfxuDDwDHREnBSPkb4UBmmb+6Gt FdgDRwgjCgOMhEArphbM63g4P0EpRMHn2D4NpB0aGak7hoLe0BcxIO7DggqAlq9nHd0trG0+tly GYq4mQwAESTwe0xG5HWgXctEdhopEbuDKUsCpwu9NqwwWzD+kwbeEFgSq91UXKnuQMoj1t5h1IX 9ecI0kPx8x/0sie/0CFsyYMwlJg0xYeGUIjjWTdCj5+AqUDriGeM3CMvm7DHg/Z+BySzegayL/H GIUm8ag2h3CmtW2rGAIuO0cQEKeYPVaW7CZ/OHXdmW7UCUtbvRfGO7vBAO6PzF2mZn35YTQmzsZ 8U0YPIkvIhgi9+T2D8W3pEdGU2yMyH9uDw5vBlZBXN/enW5w== X-Received: by 2002:a05:600c:8b44:b0:492:5bb6:6d4b with SMTP id 5b1f17b1804b1-493c2ba5968mr15479995e9.34.1782899559390; Wed, 01 Jul 2026 02:52:39 -0700 (PDT) Received: from pumpkin (host-92-21-50-228.as13285.net. [92.21.50.228]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-493be4d8f5asm93522145e9.8.2026.07.01.02.52.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jul 2026 02:52:39 -0700 (PDT) Date: Wed, 1 Jul 2026 10:52:37 +0100 From: David Laight To: Ian Bridges Cc: Paul Moore , Stephen Smalley , Ondrej Mosnacek , selinux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH v2] selinux: replace strlcat() with seq_buf in selinux_ima_collect_state() Message-ID: <20260701105237.0a797d5c@pumpkin> In-Reply-To: References: <20260627224914.68b506b0@pumpkin> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 1 Jul 2026 00:54:11 -0500 Ian Bridges 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 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 > > > #include > > > +#include > > > #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; > > > } > >