Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default
From: Jarkko Sakkinen @ 2025-09-18 19:12 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: linux-integrity, stable, Chris Fenner, Peter Huewe,
	Jason Gunthorpe, David Howells, Paul Moore, James Morris,
	Serge E. Hallyn, James Bottomley, open list,
	open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <aMxV9fB0E72QQY2G@earth.li>

On Thu, Sep 18, 2025 at 07:56:53PM +0100, Jonathan McDowell wrote:
> On Mon, Aug 25, 2025 at 11:32:23PM +0300, Jarkko Sakkinen wrote:
> > After reading all the feedback, right now disabling the TPM2_TCG_HMAC
> > is the right call.
> > 
> > Other views discussed:
> > 
> > A. Having a kernel command-line parameter or refining the feature
> >   otherwise. This goes to the area of improvements.  E.g., one
> >   example is my own idea where the null key specific code would be
> >   replaced with a persistent handle parameter (which can be
> >   *unambigously* defined as part of attestation process when
> >   done correctly).
> > 
> > B. Removing the code. I don't buy this because that is same as saying
> >   that HMAC encryption cannot work at all (if really nitpicking) in
> >   any form. Also I disagree on the view that the feature could not
> >   be refined to something more reasoable.
> > 
> > Also, both A and B are worst options in terms of backporting.
> > 
> > Thus, this is the best possible choice.
> 
> I think this is reasonable; it's adding runtime overhead and not adding
> enough benefit to be the default upstream.

Yes, I think this is a balanced change. I agree what you say and at the
same time this gives more space to refine it something usable. Right now
it is much harder to tackle those issue, as it is part of the default
config. By looking at things from this angle, the change is also
benefical for the feature itself (in the long run).

> Reviewed-By: Jonathan McDowell <noodles@earth.li>

Thank you! I appreciate this and will append this to the commit.

BR, Jarkko

^ permalink raw reply

* [PATCH] tpm: use a map for tpm2_calc_ordinal_duration()
From: Jarkko Sakkinen @ 2025-09-18 18:57 UTC (permalink / raw)
  To: linux-integrity
  Cc: Jarkko Sakkinen, Frédéric Jouen, Peter Huewe,
	Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
	Paul Moore, James Morris, Serge E. Hallyn, open list,
	open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM

The current shenanigans for duration calculation introduce too much
complexity for a trivial problem, and further the code is hard to patch and
maintain.

Address these issues with a flat look-up table, which is easy to understand
and patch. If leaf driver specific patching is required in future, it is
easy enough to make a copy of this table during driver initialization and
add the chip parameter back.

'chip->duration' is retained for TPM 1.x.

As the first entry for this new behavior address TCG spec update mentioned
in this issue:

https://github.com/raspberrypi/linux/issues/7054

Therefore, for TPM_SelfTest the duration is set to 3000 ms.

This does not categorize a as bug, given that this is introduced to the
spec after the feature was originally made.

Cc: Frédéric Jouen <fjouen@sealsq.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm-interface.c |   2 +-
 drivers/char/tpm/tpm.h           |   2 +-
 drivers/char/tpm/tpm2-cmd.c      | 115 +++++++++----------------------
 3 files changed, 34 insertions(+), 85 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index b71725827743..c9f173001d0e 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr,
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 {
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return tpm2_calc_ordinal_duration(chip, ordinal);
+		return tpm2_calc_ordinal_duration(ordinal);
 	else
 		return tpm1_calc_ordinal_duration(chip, ordinal);
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7bb87fa5f7a1..2726bd38e5ac 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
-unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
 int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 524d802ede26..29c0d6a8ec20 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -48,100 +48,49 @@ int tpm2_get_timeouts(struct tpm_chip *chip)
 	return 0;
 }
 
-/**
- * tpm2_ordinal_duration_index() - returns an index to the chip duration table
- * @ordinal: TPM command ordinal.
- *
- * The function returns an index to the chip duration table
- * (enum tpm_duration), that describes the maximum amount of
- * time the chip could take to return the result for a  particular ordinal.
- *
- * The values of the MEDIUM, and LONG durations are taken
- * from the PC Client Profile (PTP) specification (750, 2000 msec)
- *
- * LONG_LONG is for commands that generates keys which empirically takes
- * a longer time on some systems.
- *
- * Return:
- * * TPM_MEDIUM
- * * TPM_LONG
- * * TPM_LONG_LONG
- * * TPM_UNDEFINED
+/*
+ * Contains the maximum durations in milliseconds for TPM2 commands.
  */
-static u8 tpm2_ordinal_duration_index(u32 ordinal)
-{
-	switch (ordinal) {
-	/* Startup */
-	case TPM2_CC_STARTUP:                 /* 144 */
-		return TPM_MEDIUM;
-
-	case TPM2_CC_SELF_TEST:               /* 143 */
-		return TPM_LONG;
-
-	case TPM2_CC_GET_RANDOM:              /* 17B */
-		return TPM_LONG;
-
-	case TPM2_CC_SEQUENCE_UPDATE:         /* 15C */
-		return TPM_MEDIUM;
-	case TPM2_CC_SEQUENCE_COMPLETE:       /* 13E */
-		return TPM_MEDIUM;
-	case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
-		return TPM_MEDIUM;
-	case TPM2_CC_HASH_SEQUENCE_START:     /* 186 */
-		return TPM_MEDIUM;
-
-	case TPM2_CC_VERIFY_SIGNATURE:        /* 177 */
-		return TPM_LONG_LONG;
-
-	case TPM2_CC_PCR_EXTEND:              /* 182 */
-		return TPM_MEDIUM;
-
-	case TPM2_CC_HIERARCHY_CONTROL:       /* 121 */
-		return TPM_LONG;
-	case TPM2_CC_HIERARCHY_CHANGE_AUTH:   /* 129 */
-		return TPM_LONG;
-
-	case TPM2_CC_GET_CAPABILITY:          /* 17A */
-		return TPM_MEDIUM;
-
-	case TPM2_CC_NV_READ:                 /* 14E */
-		return TPM_LONG;
-
-	case TPM2_CC_CREATE_PRIMARY:          /* 131 */
-		return TPM_LONG_LONG;
-	case TPM2_CC_CREATE:                  /* 153 */
-		return TPM_LONG_LONG;
-	case TPM2_CC_CREATE_LOADED:           /* 191 */
-		return TPM_LONG_LONG;
-
-	default:
-		return TPM_UNDEFINED;
-	}
-}
+static const struct {
+	unsigned long ordinal;
+	unsigned long duration;
+} tpm2_ordinal_duration_map[] = {
+	{TPM2_CC_STARTUP, 750},
+	{TPM2_CC_SELF_TEST, 3000},
+	{TPM2_CC_GET_RANDOM, 2000},
+	{TPM2_CC_SEQUENCE_UPDATE, 750},
+	{TPM2_CC_SEQUENCE_COMPLETE, 750},
+	{TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
+	{TPM2_CC_HASH_SEQUENCE_START, 750},
+	{TPM2_CC_VERIFY_SIGNATURE, 30000},
+	{TPM2_CC_PCR_EXTEND, 750},
+	{TPM2_CC_HIERARCHY_CONTROL, 2000},
+	{TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
+	{TPM2_CC_GET_CAPABILITY, 750},
+	{TPM2_CC_NV_READ, 2000},
+	{TPM2_CC_CREATE_PRIMARY, 30000},
+	{TPM2_CC_CREATE, 30000},
+	{TPM2_CC_CREATE_LOADED, 30000},
+};
 
 /**
- * tpm2_calc_ordinal_duration() - calculate the maximum command duration
- * @chip:    TPM chip to use.
+ * tpm2_calc_ordinal_duration() - Calculate the maximum command duration
  * @ordinal: TPM command ordinal.
  *
- * The function returns the maximum amount of time the chip could take
- * to return the result for a particular ordinal in jiffies.
- *
- * Return: A maximal duration time for an ordinal in jiffies.
+ * Returns the maximum amount of time the chip is expected by kernel to
+ * take in jiffies.
  */
-unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
+unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
 {
-	unsigned int index;
+	int i;
 
-	index = tpm2_ordinal_duration_index(ordinal);
+	for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
+		if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
+			return tpm2_ordinal_duration_map[i].duration;
 
-	if (index != TPM_UNDEFINED)
-		return chip->duration[index];
-	else
-		return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
+	return TPM2_DURATION_DEFAULT;
 }
 
-
 struct tpm2_pcr_read_out {
 	__be32	update_cnt;
 	__be32	pcr_selects_cnt;
-- 
2.39.5


^ permalink raw reply related

* Re: [PATCH] tpm: Disable TPM2_TCG_HMAC by default
From: Jonathan McDowell @ 2025-09-18 18:56 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, stable, Chris Fenner, Peter Huewe,
	Jason Gunthorpe, David Howells, Paul Moore, James Morris,
	Serge E. Hallyn, James Bottomley, open list,
	open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250825203223.629515-1-jarkko@kernel.org>

On Mon, Aug 25, 2025 at 11:32:23PM +0300, Jarkko Sakkinen wrote:
>After reading all the feedback, right now disabling the TPM2_TCG_HMAC
>is the right call.
>
>Other views discussed:
>
>A. Having a kernel command-line parameter or refining the feature
>   otherwise. This goes to the area of improvements.  E.g., one
>   example is my own idea where the null key specific code would be
>   replaced with a persistent handle parameter (which can be
>   *unambigously* defined as part of attestation process when
>   done correctly).
>
>B. Removing the code. I don't buy this because that is same as saying
>   that HMAC encryption cannot work at all (if really nitpicking) in
>   any form. Also I disagree on the view that the feature could not
>   be refined to something more reasoable.
>
>Also, both A and B are worst options in terms of backporting.
>
>Thus, this is the best possible choice.

I think this is reasonable; it's adding runtime overhead and not adding 
enough benefit to be the default upstream.

Reviewed-By: Jonathan McDowell <noodles@earth.li>

>Cc: stable@vger.kernel.or # v6.10+
>Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
>Suggested-by: Chris Fenner <cfenn@google.com>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>PS. I did not post this last week because that would have been most
>likely the most counter-productive action to taken. It's better
>sometimes to take a bit of time to think (which can be seen that
>I've given also more reasonable weight to my own eaerlier
>proposals).
>
>I also accept further changes, if there is e.g., inconsistency
>with TCG_TPM_HMAC setting or similar (obviously).
>---
> drivers/char/tpm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>index dddd702b2454..3e4684f6b4af 100644
>--- a/drivers/char/tpm/Kconfig
>+++ b/drivers/char/tpm/Kconfig
>@@ -29,7 +29,7 @@ if TCG_TPM
>
> config TCG_TPM2_HMAC
> 	bool "Use HMAC and encrypted transactions on the TPM bus"
>-	default X86_64
>+	default n
> 	select CRYPTO_ECDH
> 	select CRYPTO_LIB_AESCFB
> 	select CRYPTO_LIB_SHA256
>-- 
>2.39.5

J.

-- 
] https://www.earth.li/~noodles/ []  Is this real - that's the first   [
]  PGP/GPG Key @ the.earth.li    []    thing I think every morning.    [
] via keyserver, web or email.   []                                    [
] RSA: 4096/0x94FA372B2DA8B985   []                                    [

^ permalink raw reply

* あたま専門のもみほぐし店 収益性等/概要資料
From: ヘッドミント @ 2025-09-18 11:41 UTC (permalink / raw)
  To: linux-security-module

お世話になります。


コンパクトにスタートできて手堅く収益をあげることのできる
フランチャイズビジネスの事業概要資料をご案内申し上げます。


    小資本/小スペース/少人数の
    コンパクト・フランチャイズ

     ドライヘッドスパ専門店
       “ヘッドミント”

       ・収益モデル
       ・開業に必要な資金
       ・ロイヤリティ
       ・スケジュール etc
      ↓  FC事業概要資料  ↓
      https://dryheadspa-hm.biz/fc/


ドライヘッドスパとは ――― 水を使わないヘッドスパです。


足つぼや耳つぼなど、専門のもみほぐし店がありますが
ご紹介するサロンは「 頭に特化したもみほぐし店 」です。


ドライヘッドスパというジャンルの認知度は、
現時点ではそれほど高くありません。


にも関わらず、私どもがフランチャイズ展開する“ヘッドミント”の
店舗には月間450人以上の新規客が来店し、満席が続いています。


これから先、認知度が高まることで
爆発的に伸びるポテンシャルを秘めています。


フランチャイズによる事業を展開していますので、
新たな収益づくりをお考えの方は、まずは概要資料をご覧ください。


     ドライヘッドスパ専門店
        ヘッドミント
   
      ↓  FC事業概要資料  ↓
      https://dryheadspa-hm.biz/fc/


よろしくお願いします。


------------------------------------------
 株式会社じむや
 愛知県名古屋市中区大須3-26-41堀田ビル
 TEL:052-263-4688
------------------------------------------
 本情報がご不要な方にはご迷惑をおかけし申し訳ございません。
 メールマガジンの解除は、下記URLにて承っております。
  https://dryheadspa-hm.biz/mail/
 お手数お掛けしますがよろしくお願いします。

^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Nathan Chancellor @ 2025-09-18 17:45 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Neeraj Upadhyay,
	Nick Desaulniers, Steven Rostedt, Tetsuo Handa, Thomas Gleixner,
	Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
	linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
	linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918141511.GA30263@lst.de>

On Thu, Sep 18, 2025 at 04:15:11PM +0200, Christoph Hellwig wrote:
> On Thu, Sep 18, 2025 at 03:59:11PM +0200, Marco Elver wrote:
> > A Clang version that supports `-Wthread-safety-pointer` and the new
> > alias-analysis of capability pointers is required (from this version
> > onwards):
> > 
> > 	https://github.com/llvm/llvm-project/commit/b4c98fcbe1504841203e610c351a3227f36c92a4 [3]
> 
> There's no chance to make say x86 pre-built binaries for that available?

I can use my existing kernel.org LLVM [1] build infrastructure to
generate prebuilt x86 binaries. Just give me a bit to build and upload
them. You may not be the only developer or maintainer who may want to
play with this.

[1]: https://kernel.org/pub/tools/llvm/

Cheers,
Nathan

^ permalink raw reply

* Re: [PATCH v4 22/34] lsm: group lsm_order_parse() with the other lsm_order_*() functions
From: Mimi Zohar @ 2025-09-18 17:22 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-58-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Move the lsm_order_parse() function near the other lsm_order_*()
> functions to improve readability.
> 
> No code changes.
> 
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 23/34] lsm: introduce an initcall mechanism into the LSM framework
From: Mimi Zohar @ 2025-09-18 17:19 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-59-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Currently the individual LSMs register their own initcalls, and while
> this should be harmless, it can be wasteful in the case where a LSM
> is disabled at boot as the initcall will still be executed.  This
> patch introduces support for managing the initcalls in the LSM
> framework, and future patches will convert the existing LSMs over to
> this new mechanism.
> 
> Only initcall types which are used by the current in-tree LSMs are
> supported, additional initcall types can easily be added in the future
> if needed.
> 
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 21/34] lsm: output available LSMs when debugging
From: Mimi Zohar @ 2025-09-18 17:11 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-57-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> This will display all of the LSMs built into the kernel, regardless
> of if they are enabled or not.
> 
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>



^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Ian Rogers @ 2025-09-18 16:21 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918140451.1289454-1-elver@google.com>

On Thu, Sep 18, 2025 at 7:05 AM Marco Elver <elver@google.com> wrote:
>
> Capability analysis is a C language extension, which enables statically
> checking that user-definable "capabilities" are acquired and released where
> required. An obvious application is lock-safety checking for the kernel's
> various synchronization primitives (each of which represents a "capability"),
> and checking that locking rules are not violated.
>
> Clang originally called the feature "Thread Safety Analysis" [1], with
> some terminology still using the thread-safety-analysis-only names. This
> was later changed and the feature became more flexible, gaining the
> ability to define custom "capabilities". Its foundations can be found in
> "capability systems" [2], used to specify the permissibility of
> operations to depend on some capability being held (or not held).
>
> Because the feature is not just able to express capabilities related to
> synchronization primitives, the naming chosen for the kernel departs
> from Clang's initial "Thread Safety" nomenclature and refers to the
> feature as "Capability Analysis" to avoid confusion. The implementation
> still makes references to the older terminology in some places, such as
> `-Wthread-safety` being the warning enabled option that also still
> appears in diagnostic messages.
>
> Enabling capability analysis can be seen as enabling a dialect of Linux
> C with a Capability System.
>
> Additional details can be found in the added kernel-doc documentation.
> An LWN article covered v2 of the series: https://lwn.net/Articles/1012990/
>
>  [1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
>  [2] https://www.cs.cornell.edu/talc/papers/capabilities.pdf
>
> === Development Approach ===
>
> Prior art exists in the form of Sparse's context tracking. Locking
> annotations on functions exist, so the concept of analyzing locking rules
> is not foreign to the kernel's codebase.
>
> However, Clang's analysis is more complete vs. Sparse's, with the
> typical trade-offs in static analysis: improved completeness is
> sacrificed for more possible false positives or additional annotations
> required by the programmer. Numerous options exist to disable or opt out
> certain code from analysis.
>
> This series initially aimed to retain compatibility with Sparse, which
> can provide tree-wide analysis of a subset of the capability analysis
> introduced, but it was later decided to drop Sparse compatibility. For
> the most part, the new (and old) keywords used for annotations remain
> the same, and many of the pre-existing annotations remain valid.
>
> One big question is how to enable this feature, given we end up with a
> new dialect of C -- 2 approaches have been considered:
>
>   A. Tree-wide all-or-nothing approach. This approach requires tree-wide
>      changes, adding annotations or selective opt-outs. Making additional
>      primitives capability-enabled increases churn, esp. where maintainers
>      are unaware of the feature's existence and how to use it.
>
> Because we can't change the programming language (even if from one C
> dialect to another) of the kernel overnight, a different approach might
> cause less friction.
>
>   B. A selective, incremental, and much less intrusive approach.
>      Maintainers of subsystems opt in their modules or directories into
>      "capability analysis" (via Makefile):
>
>        CAPABILITY_ANALYSIS_foo.o := y   # foo.o only
>        CAPABILITY_ANALYSIS := y         # all TUs
>
>      Most (eventually all) synchronization primitives and more
>      capabilities (including ones that could track "irq disabled",
>      "preemption" disabled, etc.) could be supported.
>
> The approach taken by this series is B. This ensures that only
> subsystems where maintainers are willing to deal with any warnings are
> opted-in. Introducing the feature can be done incrementally, without
> large tree-wide changes and adding numerous opt-outs and annotations to
> the majority of code.
>
>   Note: Bart Van Assche concurrently worked on enabling -Wthread-safety:
>   https://lore.kernel.org/all/20250206175114.1974171-1-bvanassche@acm.org/
>   Bart's work has shown what it might take to go with approach A
>   (tree-wide, restricted to 'mutex' usage). This has shown that the
>   analysis finds real issues when applied to enough subsystems!  We hope
>   this serves as motivation to eventually enable the analysis in as many
>   subsystems as possible, particularly subsystems that are not as easily
>   tested by CI systems and test robots.
>
> === Initial Uses ===
>
> With this initial series, the following synchronization primitives are
> supported: `raw_spinlock_t`, `spinlock_t`, `rwlock_t`, `mutex`,
> `seqlock_t`, `bit_spinlock`, RCU, SRCU (`srcu_struct`), `rw_semaphore`,
> `local_lock_t`, `ww_mutex`.
>
> To demonstrate use of the feature on real kernel code, the series also
> enables capability analysis for the following subsystems:
>
>         * kernel/kcov
>         * kernel/kcsan
>         * kernel/sched/
>         * lib/rhashtable
>         * lib/stackdepot
>         * mm/kfence
>         * security/tomoyo
>         * crypto/
>
> The initial benefits are static detection of violations of locking
> rules. As more capabilities are added, we would see more static checking
> beyond what regular C can provide, all while remaining easy (read quick)
> to use via the Clang compiler.
>
>   Note: The kernel already provides dynamic analysis tools Lockdep and
>   KCSAN for lock-safety checking and data-race detection respectively.
>   Unlike those, Clang's capability analysis is a compile-time static
>   analysis with no runtime impact. The static analysis complements
>   existing dynamic analysis tools, as it may catch some issues before
>   even getting into a running kernel, but is *not* a replacement for
>   whole-kernel testing with the dynamic analysis tools enabled!
>
> === Appendix ===
>
> A Clang version that supports `-Wthread-safety-pointer` and the new
> alias-analysis of capability pointers is required (from this version
> onwards):
>
>         https://github.com/llvm/llvm-project/commit/b4c98fcbe1504841203e610c351a3227f36c92a4 [3]
>
> This series is also available at this Git tree:
>
>         https://git.kernel.org/pub/scm/linux/kernel/git/melver/linux.git/log/?h=cap-analysis/dev
>
> === Changelog ===
>
> v3:
>
>   - Bump min. Clang version to 22+ (unreleased), which now supports:
>
>         * re-entrancy via __attribute__((reentrant_capability));
>         * basic form of capability alias analysis [3] - which is the
>           biggest improvement since v2.
>
>     This was the result of conclusions from this discussion:
>     https://lore.kernel.org/all/CANpmjNPquO=W1JAh1FNQb8pMQjgeZAKCPQUAd7qUg=5pjJ6x=Q@mail.gmail.com/
>
>   - Rename __asserts_cap/__assert_cap to __assumes_cap/__assume_cap.
>
>   - Switch to DECLARE_LOCK_GUARD_1_ATTRS().
>
>   - Add __acquire_ret and __acquire_shared_ret helper macros - can be
>     used to define function-like macros that return objects which
>     contains a held capabilities. Works now because of capability alias
>     analysis.
>
>   - Add capability_unsafe_alias() helper, where the analysis rightfully
>     points out we're doing strange things with aliases but we don't
>     care.
>
>   - Support multi-argument attributes.
>
>   - Enable for kernel/sched/{core,fair}.c, kernel/kcsan.
>   - Drop drivers/tty changes (revisit later).
>
> v2: https://lore.kernel.org/all/20250304092417.2873893-1-elver@google.com/
>
>   - Remove Sparse context tracking support - after the introduction of
>     Clang support, so that backports can skip removal of Sparse support.
>
>   - Remove __cond_lock() function-like helper.
>
>   - ww_mutex support.
>
>   - -Wthread-safety-addressof was reworked and committed in upstream
>     Clang as -Wthread-safety-pointer.
>
>   - Make __cond_acquires() and __cond_acquires_shared() take abstract
>     value, since compiler only cares about zero and non-zero.
>
>   - Rename __var_guarded_by to simply __guarded_by. Initially the idea
>     was to be explicit about if the variable itself or the pointed-to
>     data is guarded, but in the long-term, making this shorter might be
>     better.
>
>   - Likewise rename __ref_guarded_by to __pt_guarded_by.
>
>   - Introduce common header warning suppressions - this is a better
>     solution than guarding header inclusions with disable_ +
>     enable_capability_analysis(). Header suppressions are disabled when
>     selecting CONFIG_WARN_CAPABILITY_ANALYSIS_ALL=y. This bumps the
>     minimum Clang version required to 20+.
>
>   - Make the data_race() macro imply disabled capability analysis.
>     Writing capability_unsafe(data_race(..)) is unnecessarily verbose
>     and data_race() on its own already indicates something subtly unsafe
>     is happening.  This change was made after analysis of a finding in
>     security/tomoyo.
>
>   - Enable analysis in the following subsystems as additional examples
>     of larger subsystem. Where it was obvious, the __guarded_by
>     attribute was added to lock-guarded variables to improve coverage.
>
>         * drivers/tty
>         * security/tomoyo
>         * crypto/
>
> RFC v1: https://lore.kernel.org/lkml/20250206181711.1902989-1-elver@google.com

Thanks for this and lgtm. Fwiw, there is already thread safety
analysis in tools/perf:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/mutex.h?h=perf-tools-next#n43
and we should migrate that code to use this code.

Something that I've wondered about capabilities is to use them for
detecting missing reference count "puts", which feel similar to missed
unlocks. In my experience the sanitizers are weak in this area as in
C++ you can trivially use RAII, however, frustratingly clang's
capability analysis is disabled in C++'s constructors and destructors
(not an issue here :-) and based on my rusty memory). To solve this
for perf (and fix many many bugs) we did a form of runtime RAII:
https://perfwiki.github.io/main/reference-count-checking/
There is likely something better than can be done with the nearly RAII
that is/are cleanups. Trying to make that sane for a data-structure
like an rbtree is hard and maybe rust is just the only solution there.
Anyway, it is great to see thread safety analysis pushed forward.

Thanks,
Ian

> Marco Elver (35):
>   compiler_types: Move lock checking attributes to
>     compiler-capability-analysis.h
>   compiler-capability-analysis: Add infrastructure for Clang's
>     capability analysis
>   compiler-capability-analysis: Add test stub
>   Documentation: Add documentation for Compiler-Based Capability
>     Analysis
>   checkpatch: Warn about capability_unsafe() without comment
>   cleanup: Basic compatibility with capability analysis
>   lockdep: Annotate lockdep assertions for capability analysis
>   locking/rwlock, spinlock: Support Clang's capability analysis
>   compiler-capability-analysis: Change __cond_acquires to take return
>     value
>   locking/mutex: Support Clang's capability analysis
>   locking/seqlock: Support Clang's capability analysis
>   bit_spinlock: Include missing <asm/processor.h>
>   bit_spinlock: Support Clang's capability analysis
>   rcu: Support Clang's capability analysis
>   srcu: Support Clang's capability analysis
>   kref: Add capability-analysis annotations
>   locking/rwsem: Support Clang's capability analysis
>   locking/local_lock: Include missing headers
>   locking/local_lock: Support Clang's capability analysis
>   locking/ww_mutex: Support Clang's capability analysis
>   debugfs: Make debugfs_cancellation a capability struct
>   compiler-capability-analysis: Remove Sparse support
>   compiler-capability-analysis: Remove __cond_lock() function-like
>     helper
>   compiler-capability-analysis: Introduce header suppressions
>   compiler: Let data_race() imply disabled capability analysis
>   MAINTAINERS: Add entry for Capability Analysis
>   kfence: Enable capability analysis
>   kcov: Enable capability analysis
>   kcsan: Enable capability analysis
>   stackdepot: Enable capability analysis
>   rhashtable: Enable capability analysis
>   printk: Move locking annotation to printk.c
>   security/tomoyo: Enable capability analysis
>   crypto: Enable capability analysis
>   sched: Enable capability analysis for core.c and fair.c
>
>  .../dev-tools/capability-analysis.rst         | 148 +++++
>  Documentation/dev-tools/index.rst             |   1 +
>  Documentation/dev-tools/sparse.rst            |  19 -
>  Documentation/mm/process_addrs.rst            |   6 +-
>  MAINTAINERS                                   |  11 +
>  Makefile                                      |   1 +
>  crypto/Makefile                               |   2 +
>  crypto/acompress.c                            |   6 +-
>  crypto/algapi.c                               |   2 +
>  crypto/api.c                                  |   1 +
>  crypto/crypto_engine.c                        |   2 +-
>  crypto/drbg.c                                 |   5 +
>  crypto/internal.h                             |   2 +-
>  crypto/proc.c                                 |   3 +
>  crypto/scompress.c                            |  24 +-
>  .../net/wireless/intel/iwlwifi/iwl-trans.c    |   4 +-
>  .../net/wireless/intel/iwlwifi/iwl-trans.h    |   6 +-
>  .../intel/iwlwifi/pcie/gen1_2/internal.h      |   5 +-
>  .../intel/iwlwifi/pcie/gen1_2/trans.c         |   4 +-
>  fs/dlm/lock.c                                 |   2 +-
>  include/crypto/internal/acompress.h           |   7 +-
>  include/crypto/internal/engine.h              |   2 +-
>  include/linux/bit_spinlock.h                  |  24 +-
>  include/linux/cleanup.h                       |  17 +
>  include/linux/compiler-capability-analysis.h  | 423 +++++++++++++
>  include/linux/compiler.h                      |   2 +
>  include/linux/compiler_types.h                |  18 +-
>  include/linux/console.h                       |   4 +-
>  include/linux/debugfs.h                       |  12 +-
>  include/linux/kref.h                          |   2 +
>  include/linux/list_bl.h                       |   2 +
>  include/linux/local_lock.h                    |  45 +-
>  include/linux/local_lock_internal.h           |  73 ++-
>  include/linux/lockdep.h                       |  12 +-
>  include/linux/mm.h                            |  33 +-
>  include/linux/mutex.h                         |  35 +-
>  include/linux/mutex_types.h                   |   4 +-
>  include/linux/rcupdate.h                      |  86 +--
>  include/linux/refcount.h                      |   6 +-
>  include/linux/rhashtable.h                    |  14 +-
>  include/linux/rwlock.h                        |  22 +-
>  include/linux/rwlock_api_smp.h                |  43 +-
>  include/linux/rwlock_rt.h                     |  44 +-
>  include/linux/rwlock_types.h                  |  10 +-
>  include/linux/rwsem.h                         |  66 +-
>  include/linux/sched.h                         |   6 +-
>  include/linux/sched/signal.h                  |  16 +-
>  include/linux/sched/task.h                    |   5 +-
>  include/linux/sched/wake_q.h                  |   3 +
>  include/linux/seqlock.h                       |  24 +
>  include/linux/seqlock_types.h                 |   5 +-
>  include/linux/spinlock.h                      |  89 ++-
>  include/linux/spinlock_api_smp.h              |  34 +-
>  include/linux/spinlock_api_up.h               | 112 +++-
>  include/linux/spinlock_rt.h                   |  37 +-
>  include/linux/spinlock_types.h                |  10 +-
>  include/linux/spinlock_types_raw.h            |   5 +-
>  include/linux/srcu.h                          |  60 +-
>  include/linux/srcutiny.h                      |   4 +
>  include/linux/srcutree.h                      |   6 +-
>  include/linux/ww_mutex.h                      |  22 +-
>  kernel/Makefile                               |   2 +
>  kernel/kcov.c                                 |  36 +-
>  kernel/kcsan/Makefile                         |   2 +
>  kernel/kcsan/report.c                         |  11 +-
>  kernel/printk/printk.c                        |   2 +
>  kernel/sched/Makefile                         |   3 +
>  kernel/sched/core.c                           |  89 ++-
>  kernel/sched/fair.c                           |   9 +-
>  kernel/sched/sched.h                          | 110 +++-
>  kernel/signal.c                               |   4 +-
>  kernel/time/posix-timers.c                    |  13 +-
>  lib/Kconfig.debug                             |  45 ++
>  lib/Makefile                                  |   6 +
>  lib/dec_and_lock.c                            |   8 +-
>  lib/rhashtable.c                              |   5 +-
>  lib/stackdepot.c                              |  20 +-
>  lib/test_capability-analysis.c                | 596 ++++++++++++++++++
>  mm/kfence/Makefile                            |   2 +
>  mm/kfence/core.c                              |  20 +-
>  mm/kfence/kfence.h                            |  14 +-
>  mm/kfence/report.c                            |   4 +-
>  mm/memory.c                                   |   4 +-
>  mm/pgtable-generic.c                          |  19 +-
>  net/ipv4/tcp_sigpool.c                        |   2 +-
>  scripts/Makefile.capability-analysis          |  11 +
>  scripts/Makefile.lib                          |  10 +
>  scripts/capability-analysis-suppression.txt   |  33 +
>  scripts/checkpatch.pl                         |   8 +
>  security/tomoyo/Makefile                      |   2 +
>  security/tomoyo/common.c                      |  52 +-
>  security/tomoyo/common.h                      |  77 +--
>  security/tomoyo/domain.c                      |   1 +
>  security/tomoyo/environ.c                     |   1 +
>  security/tomoyo/file.c                        |   5 +
>  security/tomoyo/gc.c                          |  28 +-
>  security/tomoyo/mount.c                       |   2 +
>  security/tomoyo/network.c                     |   3 +
>  tools/include/linux/compiler_types.h          |   2 -
>  99 files changed, 2370 insertions(+), 589 deletions(-)
>  create mode 100644 Documentation/dev-tools/capability-analysis.rst
>  create mode 100644 include/linux/compiler-capability-analysis.h
>  create mode 100644 lib/test_capability-analysis.c
>  create mode 100644 scripts/Makefile.capability-analysis
>  create mode 100644 scripts/capability-analysis-suppression.txt
>
> --
> 2.51.0.384.g4c02a37b29-goog
>

^ permalink raw reply

* Re: [PATCH v3 02/35] compiler-capability-analysis: Add infrastructure for Clang's capability analysis
From: Steven Rostedt @ 2025-09-18 16:14 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Ian Rogers, Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar,
	Will Deacon, David S. Miller, Luc Van Oostenryck,
	Paul E. McKenney, Alexander Potapenko, Arnd Bergmann,
	Bill Wendling, Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Tetsuo Handa, Thomas Gleixner,
	Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
	linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
	linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <1ca90ba0-7bdc-43d1-af12-bba73dd3234a@acm.org>

On Thu, 18 Sep 2025 09:03:17 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> On 9/18/25 8:58 AM, Ian Rogers wrote:
> > On Thu, Sep 18, 2025 at 7:05 AM Marco Elver <elver@google.com> wrote:  
> >> +config WARN_CAPABILITY_ANALYSIS
> >> +       bool "Compiler capability-analysis warnings"
> >> +       depends on CC_IS_CLANG && CLANG_VERSION >= 220000
> >> +       # Branch profiling re-defines "if", which messes with the compiler's
> >> +       # ability to analyze __cond_acquires(..), resulting in false positives.
> >> +       depends on !TRACE_BRANCH_PROFILING  
> > 
> > Err, wow! What and huh, and why? Crikes. I'm amazed you found such an
> > option exists. I must be very naive to have never heard of it and now
> > I wonder if it is needed and load bearing?  
> 
> (+Steven)
> 
> This is an old option. I think this commit introduced it:
> 
> commit 52f232cb720a7babb752849cbc2cab2d24021209
> Author: Steven Rostedt <rostedt@goodmis.org>
> Date:   Wed Nov 12 00:14:40 2008 -0500
> 
>      tracing: likely/unlikely branch annotation tracer
> 

I still use it every year (enable it during December for a few weeks on my
workstation and server) and post the results publicly. When I get time, I
try to fix (add / remove) likely/unlikely statements. Unfortunately, I
haven't had time to look deeper at them.

 https://rostedt.org/branches/current/

This year I missed December and ended up running it in January instead.

-- Steve

^ permalink raw reply

* Re: [PATCH v3 02/35] compiler-capability-analysis: Add infrastructure for Clang's capability analysis
From: Bart Van Assche @ 2025-09-18 16:03 UTC (permalink / raw)
  To: Ian Rogers, Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bill Wendling,
	Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu,
	Steven Rostedt
In-Reply-To: <CAP-5=fUfbMAKrLC_z04o9r0kGZ02tpHfv8cOecQAQaYPx44awA@mail.gmail.com>

On 9/18/25 8:58 AM, Ian Rogers wrote:
> On Thu, Sep 18, 2025 at 7:05 AM Marco Elver <elver@google.com> wrote:
>> +config WARN_CAPABILITY_ANALYSIS
>> +       bool "Compiler capability-analysis warnings"
>> +       depends on CC_IS_CLANG && CLANG_VERSION >= 220000
>> +       # Branch profiling re-defines "if", which messes with the compiler's
>> +       # ability to analyze __cond_acquires(..), resulting in false positives.
>> +       depends on !TRACE_BRANCH_PROFILING
> 
> Err, wow! What and huh, and why? Crikes. I'm amazed you found such an
> option exists. I must be very naive to have never heard of it and now
> I wonder if it is needed and load bearing?

(+Steven)

This is an old option. I think this commit introduced it:

commit 52f232cb720a7babb752849cbc2cab2d24021209
Author: Steven Rostedt <rostedt@goodmis.org>
Date:   Wed Nov 12 00:14:40 2008 -0500

     tracing: likely/unlikely branch annotation tracer

Bart.

^ permalink raw reply

* Re: [PATCH v3 02/35] compiler-capability-analysis: Add infrastructure for Clang's capability analysis
From: Ian Rogers @ 2025-09-18 15:58 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918140451.1289454-3-elver@google.com>

On Thu, Sep 18, 2025 at 7:05 AM Marco Elver <elver@google.com> wrote:
>
> Capability analysis is a C language extension, which enables statically
> checking that user-definable "capabilities" are acquired and released where
> required. An obvious application is lock-safety checking for the kernel's
> various synchronization primitives (each of which represents a "capability"),
> and checking that locking rules are not violated.
>
> Clang originally called the feature "Thread Safety Analysis" [1], with
> some terminology still using the thread-safety-analysis-only names. This
> was later changed and the feature became more flexible, gaining the
> ability to define custom "capabilities". Its foundations can be found in
> "capability systems", used to specify the permissibility of operations
> to depend on some capability being held (or not held).
>
> [1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
> [2] https://www.cs.cornell.edu/talc/papers/capabilities.pdf
>
> Because the feature is not just able to express capabilities related to
> synchronization primitives, the naming chosen for the kernel departs
> from Clang's initial "Thread Safety" nomenclature and refers to the
> feature as "Capability Analysis" to avoid confusion. The implementation
> still makes references to the older terminology in some places, such as
> `-Wthread-safety` being the warning enabled option that also still
> appears in diagnostic messages.
>
> See more details in the kernel-doc documentation added in this and the
> subsequent changes.
>
> Clang version 22+ is required.
>
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> v3:
> * Require Clang 22 or later (reentrant capabilities, basic alias analysis).
> * Rename __assert_cap/__asserts_cap -> __assume_cap/__assumes_cap (suggested by Peter).
> * Add __acquire_ret and __acquire_shared_ret helper macros - can be used
>   to define function-like macros that return objects which contains a
>   held capabilities. Works now because of capability alias analysis.
> * Add capability_unsafe_alias() helper, where the analysis rightfully
>   points out we're doing strange things with aliases but we don't care.
> * Support multi-argument attributes.
>
> v2:
> * New -Wthread-safety feature rename to -Wthread-safety-pointer (was
>   -Wthread-safety-addressof).
> * Introduce __capability_unsafe() function attribute.
> * Rename __var_guarded_by to simply __guarded_by. The initial idea was
>   to be explicit if the variable or pointed-to data is guarded by, but
>   having a shorter attribute name is likely better long-term.
> * Rename __ref_guarded_by to __pt_guarded_by (pointed-to guarded by).
> ---
>  Makefile                                     |   1 +
>  include/linux/compiler-capability-analysis.h | 449 ++++++++++++++++++-
>  lib/Kconfig.debug                            |  31 ++
>  scripts/Makefile.capability-analysis         |   7 +
>  scripts/Makefile.lib                         |  10 +
>  5 files changed, 491 insertions(+), 7 deletions(-)
>  create mode 100644 scripts/Makefile.capability-analysis
>
> diff --git a/Makefile b/Makefile
> index cf37b9407821..2c91730e513b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1096,6 +1096,7 @@ include-$(CONFIG_RANDSTRUCT)      += scripts/Makefile.randstruct
>  include-$(CONFIG_KSTACK_ERASE) += scripts/Makefile.kstack_erase
>  include-$(CONFIG_AUTOFDO_CLANG)        += scripts/Makefile.autofdo
>  include-$(CONFIG_PROPELLER_CLANG)      += scripts/Makefile.propeller
> +include-$(CONFIG_WARN_CAPABILITY_ANALYSIS) += scripts/Makefile.capability-analysis
>  include-$(CONFIG_GCC_PLUGINS)  += scripts/Makefile.gcc-plugins
>
>  include $(addprefix $(srctree)/, $(include-y))
> diff --git a/include/linux/compiler-capability-analysis.h b/include/linux/compiler-capability-analysis.h
> index 7546ddb83f86..6f3f185478bc 100644
> --- a/include/linux/compiler-capability-analysis.h
> +++ b/include/linux/compiler-capability-analysis.h
> @@ -6,27 +6,462 @@
>  #ifndef _LINUX_COMPILER_CAPABILITY_ANALYSIS_H
>  #define _LINUX_COMPILER_CAPABILITY_ANALYSIS_H
>
> +#if defined(WARN_CAPABILITY_ANALYSIS)
> +
> +/*
> + * The below attributes are used to define new capability types. Internal only.
> + */
> +# define __cap_type(name)                      __attribute__((capability(#name)))
> +# define __reentrant_cap                       __attribute__((reentrant_capability))
> +# define __acquires_cap(...)                   __attribute__((acquire_capability(__VA_ARGS__)))
> +# define __acquires_shared_cap(...)            __attribute__((acquire_shared_capability(__VA_ARGS__)))
> +# define __try_acquires_cap(ret, var)          __attribute__((try_acquire_capability(ret, var)))
> +# define __try_acquires_shared_cap(ret, var)   __attribute__((try_acquire_shared_capability(ret, var)))
> +# define __releases_cap(...)                   __attribute__((release_capability(__VA_ARGS__)))
> +# define __releases_shared_cap(...)            __attribute__((release_shared_capability(__VA_ARGS__)))
> +# define __assumes_cap(...)                    __attribute__((assert_capability(__VA_ARGS__)))
> +# define __assumes_shared_cap(...)             __attribute__((assert_shared_capability(__VA_ARGS__)))
> +# define __returns_cap(var)                    __attribute__((lock_returned(var)))
> +
> +/*
> + * The below are used to annotate code being checked. Internal only.
> + */
> +# define __excludes_cap(...)           __attribute__((locks_excluded(__VA_ARGS__)))
> +# define __requires_cap(...)           __attribute__((requires_capability(__VA_ARGS__)))
> +# define __requires_shared_cap(...)    __attribute__((requires_shared_capability(__VA_ARGS__)))
> +
> +/**
> + * __guarded_by - struct member and globals attribute, declares variable
> + *                protected by capability
> + *
> + * Declares that the struct member or global variable must be guarded by the
> + * given capabilities. Read operations on the data require shared access,
> + * while write operations require exclusive access.
> + *
> + * .. code-block:: c
> + *
> + *     struct some_state {
> + *             spinlock_t lock;
> + *             long counter __guarded_by(&lock);
> + *     };
> + */
> +# define __guarded_by(...)             __attribute__((guarded_by(__VA_ARGS__)))
> +
> +/**
> + * __pt_guarded_by - struct member and globals attribute, declares pointed-to
> + *                   data is protected by capability
> + *
> + * Declares that the data pointed to by the struct member pointer or global
> + * pointer must be guarded by the given capabilities. Read operations on the
> + * data require shared access, while write operations require exclusive access.
> + *
> + * .. code-block:: c
> + *
> + *     struct some_state {
> + *             spinlock_t lock;
> + *             long *counter __pt_guarded_by(&lock);
> + *     };
> + */
> +# define __pt_guarded_by(...)          __attribute__((pt_guarded_by(__VA_ARGS__)))
> +
> +/**
> + * struct_with_capability() - declare or define a capability struct
> + * @name: struct name
> + *
> + * Helper to declare or define a struct type with capability of the same name.
> + *
> + * .. code-block:: c
> + *
> + *     struct_with_capability(my_handle) {
> + *             int foo;
> + *             long bar;
> + *     };
> + *
> + *     struct some_state {
> + *             ...
> + *     };
> + *     // ... declared elsewhere ...
> + *     struct_with_capability(some_state);
> + *
> + * Note: The implementation defines several helper functions that can acquire,
> + * release, and assert the capability.
> + */
> +# define struct_with_capability(name, ...)                                                             \
> +       struct __cap_type(name) __VA_ARGS__ name;                                                       \
> +       static __always_inline void __acquire_cap(const struct name *var)                               \
> +               __attribute__((overloadable)) __no_capability_analysis __acquires_cap(var) { }          \
> +       static __always_inline void __acquire_shared_cap(const struct name *var)                        \
> +               __attribute__((overloadable)) __no_capability_analysis __acquires_shared_cap(var) { }   \
> +       static __always_inline bool __try_acquire_cap(const struct name *var, bool ret)                 \
> +               __attribute__((overloadable)) __no_capability_analysis __try_acquires_cap(1, var)       \
> +       { return ret; }                                                                                 \
> +       static __always_inline bool __try_acquire_shared_cap(const struct name *var, bool ret)          \
> +               __attribute__((overloadable)) __no_capability_analysis __try_acquires_shared_cap(1, var) \
> +       { return ret; }                                                                                 \
> +       static __always_inline void __release_cap(const struct name *var)                               \
> +               __attribute__((overloadable)) __no_capability_analysis __releases_cap(var) { }          \
> +       static __always_inline void __release_shared_cap(const struct name *var)                        \
> +               __attribute__((overloadable)) __no_capability_analysis __releases_shared_cap(var) { }   \
> +       static __always_inline void __assume_cap(const struct name *var)                                \
> +               __attribute__((overloadable)) __assumes_cap(var) { }                                    \
> +       static __always_inline void __assume_shared_cap(const struct name *var)                         \
> +               __attribute__((overloadable)) __assumes_shared_cap(var) { }                             \
> +       struct name
> +
> +/**
> + * disable_capability_analysis() - disables capability analysis
> + *
> + * Disables capability analysis. Must be paired with a later
> + * enable_capability_analysis().
> + */
> +# define disable_capability_analysis()                         \
> +       __diag_push();                                          \
> +       __diag_ignore_all("-Wunknown-warning-option", "")       \
> +       __diag_ignore_all("-Wthread-safety", "")                \
> +       __diag_ignore_all("-Wthread-safety-pointer", "")
> +
> +/**
> + * enable_capability_analysis() - re-enables capability analysis
> + *
> + * Re-enables capability analysis. Must be paired with a prior
> + * disable_capability_analysis().
> + */
> +# define enable_capability_analysis() __diag_pop()
> +
> +/**
> + * __no_capability_analysis - function attribute, disables capability analysis
> + *
> + * Function attribute denoting that capability analysis is disabled for the
> + * whole function. Prefer use of `capability_unsafe()` where possible.
> + */
> +# define __no_capability_analysis      __attribute__((no_thread_safety_analysis))
> +
> +#else /* !WARN_CAPABILITY_ANALYSIS */
> +
> +# define __cap_type(name)
> +# define __reentrant_cap
> +# define __acquires_cap(...)
> +# define __acquires_shared_cap(...)
> +# define __try_acquires_cap(ret, var)
> +# define __try_acquires_shared_cap(ret, var)
> +# define __releases_cap(...)
> +# define __releases_shared_cap(...)
> +# define __assumes_cap(...)
> +# define __assumes_shared_cap(...)
> +# define __returns_cap(var)
> +# define __guarded_by(...)
> +# define __pt_guarded_by(...)
> +# define __excludes_cap(...)
> +# define __requires_cap(...)
> +# define __requires_shared_cap(...)
> +# define __acquire_cap(var)                    do { } while (0)
> +# define __acquire_shared_cap(var)             do { } while (0)
> +# define __try_acquire_cap(var, ret)           (ret)
> +# define __try_acquire_shared_cap(var, ret)    (ret)
> +# define __release_cap(var)                    do { } while (0)
> +# define __release_shared_cap(var)             do { } while (0)
> +# define __assume_cap(var)                     do { (void)(var); } while (0)
> +# define __assume_shared_cap(var)              do { (void)(var); } while (0)
> +# define struct_with_capability(name, ...)     struct __VA_ARGS__ name
> +# define disable_capability_analysis()
> +# define enable_capability_analysis()
> +# define __no_capability_analysis
> +
> +#endif /* WARN_CAPABILITY_ANALYSIS */
> +
> +/**
> + * capability_unsafe() - disable capability checking for contained code
> + *
> + * Disables capability checking for contained statements or expression.
> + *
> + * .. code-block:: c
> + *
> + *     struct some_data {
> + *             spinlock_t lock;
> + *             int counter __guarded_by(&lock);
> + *     };
> + *
> + *     int foo(struct some_data *d)
> + *     {
> + *             // ...
> + *             // other code that is still checked ...
> + *             // ...
> + *             return capability_unsafe(d->counter);
> + *     }
> + */
> +#define capability_unsafe(...)         \
> +({                                     \
> +       disable_capability_analysis();  \
> +       __VA_ARGS__;                    \
> +       enable_capability_analysis()    \
> +})
> +
> +/**
> + * __capability_unsafe() - function attribute, disable capability checking
> + * @comment: comment explaining why opt-out is safe
> + *
> + * Function attribute denoting that capability analysis is disabled for the
> + * whole function. Forces adding an inline comment as argument.
> + */
> +#define __capability_unsafe(comment) __no_capability_analysis
> +
> +/**
> + * capability_unsafe_alias() - helper to insert a capability "alias barrier"
> + * @p: pointer aliasing a capability or object containing capabilities
> + *
> + * No-op function that acts as a "capability alias barrier", where the analysis
> + * rightfully detects that we're switching aliases, but the switch is considered
> + * safe but beyond the analysis reasoning abilities.
> + *
> + * This should be inserted before the first use of such an alias.
> + *
> + * Implementation Note: The compiler ignores aliases that may be reassigned but
> + * their value cannot be determined (e.g. when passing a non-const pointer to an
> + * alias as a function argument).
> + */
> +#define capability_unsafe_alias(p) _capability_unsafe_alias((void **)&(p))
> +static inline void _capability_unsafe_alias(void **p) { }
> +
> +/**
> + * token_capability() - declare an abstract global capability instance
> + * @name: token capability name
> + *
> + * Helper that declares an abstract global capability instance @name that can be
> + * used as a token capability, but not backed by a real data structure (linker
> + * error if accidentally referenced). The type name is `__capability_@name`.
> + */
> +#define token_capability(name, ...)                                    \
> +       struct_with_capability(__capability_##name, ##__VA_ARGS__) {};  \
> +       extern const struct __capability_##name *name
> +
> +/**
> + * token_capability_instance() - declare another instance of a global capability
> + * @cap: token capability previously declared with token_capability()
> + * @name: name of additional global capability instance
> + *
> + * Helper that declares an additional instance @name of the same token
> + * capability class @name. This is helpful where multiple related token
> + * capabilities are declared, as it also allows using the same underlying type
> + * (`__capability_@cap`) as function arguments.
> + */
> +#define token_capability_instance(cap, name)           \
> +       extern const struct __capability_##cap *name
> +
> +/*
> + * Common keywords for static capability analysis. Both Clang's capability
> + * analysis and Sparse's context tracking are currently supported.
> + */
>  #ifdef __CHECKER__
>
>  /* Sparse context/lock checking support. */
>  # define __must_hold(x)                __attribute__((context(x,1,1)))
> +# define __must_not_hold(x)
>  # define __acquires(x)         __attribute__((context(x,0,1)))
>  # define __cond_acquires(x)    __attribute__((context(x,0,-1)))
>  # define __releases(x)         __attribute__((context(x,1,0)))
>  # define __acquire(x)          __context__(x,1)
>  # define __release(x)          __context__(x,-1)
>  # define __cond_lock(x, c)     ((c) ? ({ __acquire(x); 1; }) : 0)
> +/* For Sparse, there's no distinction between exclusive and shared locks. */
> +# define __must_hold_shared    __must_hold
> +# define __acquires_shared     __acquires
> +# define __cond_acquires_shared __cond_acquires
> +# define __releases_shared     __releases
> +# define __acquire_shared      __acquire
> +# define __release_shared      __release
> +# define __cond_lock_shared    __cond_acquire
>
>  #else /* !__CHECKER__ */
>
> -# define __must_hold(x)
> -# define __acquires(x)
> -# define __cond_acquires(x)
> -# define __releases(x)
> -# define __acquire(x)          (void)0
> -# define __release(x)          (void)0
> -# define __cond_lock(x, c)     (c)
> +/**
> + * __must_hold() - function attribute, caller must hold exclusive capability
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the caller must hold the given capability
> + * instance @x exclusively.
> + */
> +# define __must_hold(x)                __requires_cap(x)
> +
> +/**
> + * __must_not_hold() - function attribute, caller must not hold capability
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the caller must not hold the given
> + * capability instance @x.
> + */
> +# define __must_not_hold(x)    __excludes_cap(x)
> +
> +/**
> + * __acquires() - function attribute, function acquires capability exclusively
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function acquires the given
> + * capability instance @x exclusively, but does not release it.
> + */
> +# define __acquires(x)         __acquires_cap(x)
> +
> +/**
> + * __cond_acquires() - function attribute, function conditionally
> + *                     acquires a capability exclusively
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function conditionally acquires the
> + * given capability instance @x exclusively, but does not release it.
> + */
> +# define __cond_acquires(x)    __try_acquires_cap(1, x)
> +
> +/**
> + * __releases() - function attribute, function releases a capability exclusively
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function releases the given capability
> + * instance @x exclusively. The capability must be held on entry.
> + */
> +# define __releases(x)         __releases_cap(x)
> +
> +/**
> + * __acquire() - function to acquire capability exclusively
> + * @x: capability instance pointer
> + *
> + * No-op function that acquires the given capability instance @x exclusively.
> + */
> +# define __acquire(x)          __acquire_cap(x)
> +
> +/**
> + * __release() - function to release capability exclusively
> + * @x: capability instance pointer
> + *
> + * No-op function that releases the given capability instance @x.
> + */
> +# define __release(x)          __release_cap(x)
> +
> +/**
> + * __cond_lock() - function that conditionally acquires a capability
> + *                 exclusively
> + * @x: capability instance pinter
> + * @c: boolean expression
> + *
> + * Return: result of @c
> + *
> + * No-op function that conditionally acquires capability instance @x
> + * exclusively, if the boolean expression @c is true. The result of @c is the
> + * return value, to be able to create a capability-enabled interface; for
> + * example:
> + *
> + * .. code-block:: c
> + *
> + *     #define spin_trylock(l) __cond_lock(&lock, _spin_trylock(&lock))
> + */
> +# define __cond_lock(x, c)     __try_acquire_cap(x, c)
> +
> +/**
> + * __must_hold_shared() - function attribute, caller must hold shared capability
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the caller must hold the given capability
> + * instance @x with shared access.
> + */
> +# define __must_hold_shared(x) __requires_shared_cap(x)
> +
> +/**
> + * __acquires_shared() - function attribute, function acquires capability shared
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function acquires the given
> + * capability instance @x with shared access, but does not release it.
> + */
> +# define __acquires_shared(x)  __acquires_shared_cap(x)
> +
> +/**
> + * __cond_acquires_shared() - function attribute, function conditionally
> + *                            acquires a capability shared
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function conditionally acquires the
> + * given capability instance @x with shared access, but does not release it.
> + */
> +# define __cond_acquires_shared(x) __try_acquires_shared_cap(1, x)
> +
> +/**
> + * __releases_shared() - function attribute, function releases a
> + *                       capability shared
> + * @x: capability instance pointer
> + *
> + * Function attribute declaring that the function releases the given capability
> + * instance @x with shared access. The capability must be held on entry.
> + */
> +# define __releases_shared(x)  __releases_shared_cap(x)
> +
> +/**
> + * __acquire_shared() - function to acquire capability shared
> + * @x: capability instance pointer
> + *
> + * No-op function that acquires the given capability instance @x with shared
> + * access.
> + */
> +# define __acquire_shared(x)   __acquire_shared_cap(x)
> +
> +/**
> + * __release_shared() - function to release capability shared
> + * @x: capability instance pointer
> + *
> + * No-op function that releases the given capability instance @x with shared
> + * access.
> + */
> +# define __release_shared(x)   __release_shared_cap(x)
> +
> +/**
> + * __cond_lock_shared() - function that conditionally acquires a capability
> + *                        shared
> + * @x: capability instance pinter
> + * @c: boolean expression
> + *
> + * Return: result of @c
> + *
> + * No-op function that conditionally acquires capability instance @x with shared
> + * access, if the boolean expression @c is true. The result of @c is the return
> + * value, to be able to create a capability-enabled interface.
> + */
> +# define __cond_lock_shared(x, c) __try_acquire_shared_cap(x, c)
>
>  #endif /* __CHECKER__ */
>
> +/**
> + * __acquire_ret() - helper to acquire capability of return value
> + * @call: call expression
> + * @ret_expr: acquire expression that uses __ret
> + */
> +#define __acquire_ret(call, ret_expr)          \
> +       ({                                      \
> +               __auto_type __ret = call;       \
> +               __acquire(ret_expr);            \
> +               __ret;                          \
> +       })
> +
> +/**
> + * __acquire_shared_ret() - helper to acquire capability shared of return value
> + * @call: call expression
> + * @ret_expr: acquire shared expression that uses __ret
> + */
> +#define __acquire_shared_ret(call, ret_expr)   \
> +       ({                                      \
> +               __auto_type __ret = call;       \
> +               __acquire_shared(ret_expr);     \
> +               __ret;                          \
> +       })
> +
> +/*
> + * Attributes to mark functions returning acquired capabilities. This is purely
> + * cosmetic to help readability, and should be used with the above macros as
> + * follows:
> + *
> + *   struct foo { spinlock_t lock; ... };
> + *   ...
> + *   #define myfunc(...) __acquire_ret(_myfunc(__VA_ARGS__), &__ret->lock)
> + *   struct foo *_myfunc(int bar) __acquires_ret;
> + *   ...
> + */
> +#define __acquires_ret         __no_capability_analysis
> +#define __acquires_shared_ret  __no_capability_analysis
> +
>  #endif /* _LINUX_COMPILER_CAPABILITY_ANALYSIS_H */
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index dc0e0c6ed075..57e09615f88d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -613,6 +613,37 @@ config DEBUG_FORCE_WEAK_PER_CPU
>           To ensure that generic code follows the above rules, this
>           option forces all percpu variables to be defined as weak.
>
> +config WARN_CAPABILITY_ANALYSIS
> +       bool "Compiler capability-analysis warnings"
> +       depends on CC_IS_CLANG && CLANG_VERSION >= 220000
> +       # Branch profiling re-defines "if", which messes with the compiler's
> +       # ability to analyze __cond_acquires(..), resulting in false positives.
> +       depends on !TRACE_BRANCH_PROFILING

Err, wow! What and huh, and why? Crikes. I'm amazed you found such an
option exists. I must be very naive to have never heard of it and now
I wonder if it is needed and load bearing?

Ian


> +       default y
> +       help
> +         Capability analysis is a C language extension, which enables
> +         statically checking that user-definable "capabilities" are acquired
> +         and released where required.
> +
> +         Clang's name of the feature ("Thread Safety Analysis") refers to
> +         the original name of the feature; it was later expanded to be a
> +         generic "Capability Analysis" framework.
> +
> +         Requires Clang 22 or later.
> +
> +         Produces warnings by default. Select CONFIG_WERROR if you wish to
> +         turn these warnings into errors.
> +
> +config WARN_CAPABILITY_ANALYSIS_ALL
> +       bool "Enable capability analysis for all source files"
> +       depends on WARN_CAPABILITY_ANALYSIS
> +       depends on EXPERT && !COMPILE_TEST
> +       help
> +         Enable tree-wide capability analysis. This is likely to produce a
> +         large number of false positives - enable at your own risk.
> +
> +         If unsure, say N.
> +
>  endmenu # "Compiler options"
>
>  menu "Generic Kernel Debugging Instruments"
> diff --git a/scripts/Makefile.capability-analysis b/scripts/Makefile.capability-analysis
> new file mode 100644
> index 000000000000..e137751a4c9a
> --- /dev/null
> +++ b/scripts/Makefile.capability-analysis
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +capability-analysis-cflags := -DWARN_CAPABILITY_ANALYSIS       \
> +       -fexperimental-late-parse-attributes -Wthread-safety    \
> +       -Wthread-safety-pointer -Wthread-safety-beta
> +
> +export CFLAGS_CAPABILITY_ANALYSIS := $(capability-analysis-cflags)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1d581ba5df66..e0ac273bf9eb 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -105,6 +105,16 @@ _c_flags += $(if $(patsubst n%,, \
>         -D__KCSAN_INSTRUMENT_BARRIERS__)
>  endif
>
> +#
> +# Enable capability analysis flags only where explicitly opted in.
> +# (depends on variables CAPABILITY_ANALYSIS_obj.o, CAPABILITY_ANALYSIS)
> +#
> +ifeq ($(CONFIG_WARN_CAPABILITY_ANALYSIS),y)
> +_c_flags += $(if $(patsubst n%,, \
> +               $(CAPABILITY_ANALYSIS_$(target-stem).o)$(CAPABILITY_ANALYSIS)$(if $(is-kernel-object),$(CONFIG_WARN_CAPABILITY_ANALYSIS_ALL))), \
> +               $(CFLAGS_CAPABILITY_ANALYSIS))
> +endif
> +
>  #
>  # Enable AutoFDO build flags except some files or directories we don't want to
>  # enable (depends on variables AUTOFDO_PROFILE_obj.o and AUTOFDO_PROFILE).
> --
> 2.51.0.384.g4c02a37b29-goog
>

^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Linus Torvalds @ 2025-09-18 15:49 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
	Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918140451.1289454-1-elver@google.com>

On Thu, 18 Sept 2025 at 07:05, Marco Elver <elver@google.com> wrote:
>
> Capability analysis is a C language extension, which enables statically
> checking that user-definable "capabilities" are acquired and released where
> required. An obvious application is lock-safety checking for the kernel's
> various synchronization primitives (each of which represents a "capability"),
> and checking that locking rules are not violated.
>
> Clang originally called the feature "Thread Safety Analysis" [1],

So this looks really interesting, but I absolutely *hate* the new
"capability" name.

We have existing and traditional - and very very different - meaning
of "capabilities" in the kernel, and having this thing called
"capability" is just wrong. Particularly as it then talks about
"acquiring capabilities" - which is *EXACTLY* what our lon-existing
capabilities are all about, but are something entirely and totally
different.

So please - call it something else. Even if clang then calls it
'capability analysis", within the context of a kernel, please ignore
that, and call it something that makes more sense (I don't think
"capabilities" make sense even in the context of clang, but hey,
that's _their_ choice - but we should not then take that bad choice
and run with it).

Sparse called it "context analysis", and while the "analysis" part is
debatable - sparse never did much anything clever enough to merit
calling it analysis - at least the "context" part of the name is I
think somewhat sane.

Because it's about making decisions based on the context the code runs in.

But I'm certainly not married to the "context" name either. I'd still
claim it makes more sense than "capability", but the real problem with
"capability" isn't that it doesn't make sense, it's that we already
*HAVE* that as a concept, and old and traditional use is important.

But we do use the word "context" in this context quite widely even
outside of the sparse usage, ie that's what we say when we talk about
things like locking and RCU (ie we talk about running in "process
context", or about "interrupt context" etc). That's obviously where
the sparse naming comes from - it's not like sparse made that up.

So I'm really happy to see compilers start exposing these kinds of
interfaces, and the patches look sane apart from the absolutely
horrible and unacceptable name. Really - there is no way in hell we
can call this "capability" in a kernel context.

I'd suggest just doing a search-and-replace of 's/capability/context/'
and it would already make things a ton better. But maybe there are
better names for this still?

I mean, even apart from the fact that we have an existing meaning for
"capability", just look at the documentation patch, and read the first
sentence:

  Capability analysis is a C language extension, which enables statically
  checking that user-definable "capabilities" are acquired and released where
  required.

and just from a plain English language standpoint, the word
"capability" makes zero sense. I think you even realized that, in that
you put that word in quotes, because it's _so_ nonsensical.

And if not "context", maybe some other word? But really, absolutely
*not* "capability". Because that's just crazy talk.

Please? Because other than this naming issue, I think this really is a
good idea.

           Linus

^ permalink raw reply

* Re: [PATCH v4 15/34] lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
From: Mimi Zohar @ 2025-09-18 15:55 UTC (permalink / raw)
  To: Paul Moore
  Cc: linux-security-module, linux-integrity, selinux, John Johansen,
	Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhSxookZfVQhDMdrikmFrk0VoUhEwMx0FR5DS3JfK_MY9Q@mail.gmail.com>

On Thu, 2025-09-18 at 11:38 -0400, Paul Moore wrote:
> > > +     /* Normal or "mutable" LSMs */
> 
> >   What are "mutable" LSMs?!

<snip>

> As the "or" would indicate, they are "normal" LSMs where the ordering
> is mutable, as opposed to the "first" or "last" LSMs.

Yes, of course I understand that.  Still would have been clearer adding the word
"Normal or "mutable" LSM ordering".

Mimi




^ permalink raw reply

* Re: [PATCH v4 20/34] lsm: cleanup the debug and console output in lsm_init.c
From: Paul Moore @ 2025-09-18 15:54 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-integrity, selinux, John Johansen,
	Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <4db3bb94c42f11240a880a439c7a678599d7053f.camel@linux.ibm.com>

On Thu, Sep 18, 2025 at 11:51 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > Move away from an init specific init_debug() macro to a more general
> > lsm_pr()/lsm_pr_cont()/lsm_pr_dbg() set of macros that are available
> > both before and after init.  In the process we do a number of minor
> > changes to improve the LSM initialization output and cleanup the code
> > somewhat.
> >
> > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> > Reviewed-by: John Johansen <john.johhansen@canonical.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> >  security/lsm.h      |  11 ++++
> >  security/lsm_init.c | 123 +++++++++++++++++++-------------------------
> >  security/security.c |   2 +
> >  3 files changed, 66 insertions(+), 70 deletions(-)
> >
> > diff --git a/security/lsm.h b/security/lsm.h
> > index dbe755c45e57..8dc267977ae0 100644
> > --- a/security/lsm.h
> > +++ b/security/lsm.h
> > @@ -6,9 +6,20 @@
> >  #ifndef _LSM_H_
> >  #define _LSM_H_
> >
> > +#include <linux/printk.h>
> >  #include <linux/lsm_hooks.h>
> >  #include <linux/lsm_count.h>
> >
> > +/* LSM debugging */
> > +extern bool lsm_debug;
> > +#define lsm_pr(...)          pr_info(__VA_ARGS__)
> > +#define lsm_pr_cont(...)     pr_cont(__VA_ARGS__)
> > +#define lsm_pr_dbg(...)
> >
> >                               \
> > +     do {                                                            \
> > +             if (lsm_debug)                                          \
> > +                     pr_info(__VA_ARGS__);                           \
> > +     } while (0)
>
> The existing pr_info and pr_cont themselves are #defines.  Is there a reason for
> these new "#define"?  If there is a valid reason for having these new defines,
> why aren't they simply prefixed with "lsm"?

With the creation of the lsm_pr_dbg() macro which gates the output on
the "lsm.debug" flag, I felt it was more consistent to name things
this way, even if lsm_pr() and lsm_pr_cont() are just effectively
renames of the existing pr_info() and pr_cont() symbols.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v2 3/4] arch: copy_thread: pass clone_flags as u64
From: Paul Walmsley @ 2025-09-18 15:51 UTC (permalink / raw)
  To: Simon Schuster
  Cc: Dinh Nguyen, Christian Brauner, Arnd Bergmann, Andrew Morton,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Kees Cook, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, Guo Ren, Oleg Nesterov, Jens Axboe,
	Alexander Viro, Jan Kara, Tejun Heo, Johannes Weiner,
	Michal Koutný, Paul Moore, Serge Hallyn, James Morris,
	Serge E. Hallyn, Anna-Maria Behnsen, Frederic Weisbecker,
	Thomas Gleixner, Masami Hiramatsu, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Mathieu Desnoyers,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	John Johansen, Stephen Smalley, Ondrej Mosnacek, Kentaro Takeda,
	Tetsuo Handa, Richard Henderson, Matt Turner, Vineet Gupta,
	Russell King, Catalin Marinas, Will Deacon, Brian Cain,
	Huacai Chen, WANG Xuerui, Geert Uytterhoeven, Michal Simek,
	Thomas Bogendoerfer, Jonas Bonn, Stefan Kristiansson,
	Stafford Horne, James E.J. Bottomley, Helge Deller,
	Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Andreas Larsson, Richard Weinberger, Anton Ivanov, Johannes Berg,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Chris Zankel,
	Max Filippov, linux-mm, linux-kernel, linux-riscv, linux-csky,
	linux-block, linux-fsdevel, cgroups, linux-security-module,
	linux-trace-kernel, netdev, linux-perf-users, apparmor, selinux,
	linux-alpha, linux-snps-arc, linux-arm-kernel, linux-hexagon,
	loongarch, linux-m68k, linux-mips, linux-openrisc, linux-parisc,
	linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-um
In-Reply-To: <20250901-nios2-implement-clone3-v2-3-53fcf5577d57@siemens-energy.com>

On Mon, 1 Sep 2025, Simon Schuster via B4 Relay wrote:

> From: Simon Schuster <schuster.simon@siemens-energy.com>
> 
> With the introduction of clone3 in commit 7f192e3cd316 ("fork: add
> clone3") the effective bit width of clone_flags on all architectures was
> increased from 32-bit to 64-bit, with a new type of u64 for the flags.
> However, for most consumers of clone_flags the interface was not
> changed from the previous type of unsigned long.
> 
> While this works fine as long as none of the new 64-bit flag bits
> (CLONE_CLEAR_SIGHAND and CLONE_INTO_CGROUP) are evaluated, this is still
> undesirable in terms of the principle of least surprise.
> 
> Thus, this commit fixes all relevant interfaces of the copy_thread
> function that is called from copy_process to consistently pass
> clone_flags as u64, so that no truncation to 32-bit integers occurs on
> 32-bit architectures.
> 
> Signed-off-by: Simon Schuster <schuster.simon@siemens-energy.com>

Acked-by: Paul Walmsley <pjw@kernel.org> # for RISC-V

Thanks!


- Paul

^ permalink raw reply

* Re: [PATCH v4 20/34] lsm: cleanup the debug and console output in lsm_init.c
From: Mimi Zohar @ 2025-09-18 15:50 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-56-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Move away from an init specific init_debug() macro to a more general
> lsm_pr()/lsm_pr_cont()/lsm_pr_dbg() set of macros that are available
> both before and after init.  In the process we do a number of minor
> changes to improve the LSM initialization output and cleanup the code
> somewhat.
> 
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  security/lsm.h      |  11 ++++
>  security/lsm_init.c | 123 +++++++++++++++++++-------------------------
>  security/security.c |   2 +
>  3 files changed, 66 insertions(+), 70 deletions(-)
> 
> diff --git a/security/lsm.h b/security/lsm.h
> index dbe755c45e57..8dc267977ae0 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -6,9 +6,20 @@
>  #ifndef _LSM_H_
>  #define _LSM_H_
>  
> +#include <linux/printk.h>
>  #include <linux/lsm_hooks.h>
>  #include <linux/lsm_count.h>
>  
> +/* LSM debugging */
> +extern bool lsm_debug;
> +#define lsm_pr(...)		pr_info(__VA_ARGS__)
> +#define lsm_pr_cont(...)	pr_cont(__VA_ARGS__)
> +#define lsm_pr_dbg(...)
> 
> 				\
> +	do {								\
> +		if (lsm_debug)						\
> +			pr_info(__VA_ARGS__);				\
> +	} while (0)


The existing pr_info and pr_cont themselves are #defines.  Is there a reason for
these new "#define"?  If there is a valid reason for having these new defines,
why aren't they simply prefixed with "lsm"?

Mimi

^ permalink raw reply

* Re: [PATCH v4 15/34] lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
From: Paul Moore @ 2025-09-18 15:38 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-integrity, selinux, John Johansen,
	Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <8b560b9522c1c42e26a108e2f9b2977901d73649.camel@linux.ibm.com>

On Thu, Sep 18, 2025 at 7:30 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > Rename ordered_lsm_parse() to lsm_order_parse() for the sake of
> > consistency with the other LSM initialization routines, and also
> > do some minor rework of the function.  Aside from some minor style
> > decisions, the majority of the rework involved shuffling the order
> > of the LSM_FLAG_LEGACY and LSM_ORDER_FIRST code so that the
> > LSM_FLAG_LEGACY checks are handled first; it is important to note
> > that this doesn't affect the order in which the LSMs are registered.
> >
> > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> > Reviewed-by: John Johansen <john.johhansen@canonical.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> >  security/lsm_init.c | 82 ++++++++++++++++++++-------------------------
> >  1 file changed, 37 insertions(+), 45 deletions(-)
> >
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index a314484d7c2f..7b2491120fc8 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -228,83 +228,75 @@ static void __init initialize_lsm(struct lsm_info *lsm)
> >       }
> >  }
> >
> > -/* Populate ordered LSMs list from comma-separated LSM name list. */
> > -static void __init ordered_lsm_parse(const char *order, const char *origin)
> > +/**
> > + * lsm_order_parse - Parse the comma delimited LSM list
> > + * @list: LSM list
> > + * @src: source of the list
> > + */
> > +static void __init lsm_order_parse(const char *list, const char *src)
> >  {
> >       struct lsm_info *lsm;
> >       char *sep, *name, *next;
> >
> > -     /* LSM_ORDER_FIRST is always first. */
> > -     lsm_for_each_raw(lsm) {
> > -             if (lsm->order == LSM_ORDER_FIRST)
> > -                     lsm_order_append(lsm, "  first");
> > -     }
> > -
> > -     /* Process "security=", if given. */
> > +     /* Handle any Legacy LSM exclusions if one was specified. */
> >       if (lsm_order_legacy) {
> > -             struct lsm_info *major;
> > -
> >               /*
> > -              * To match the original "security=" behavior, this
> > -              * explicitly does NOT fallback to another Legacy Major
> > -              * if the selected one was separately disabled: disable
> > -              * all non-matching Legacy Major LSMs.
> > +              * To match the original "security=" behavior, this explicitly
> > +              * does NOT fallback to another Legacy Major if the selected
> > +              * one was separately disabled: disable all non-matching
> > +              * Legacy Major LSMs.
> >                */
> > -             lsm_for_each_raw(major) {
> > -                     if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&
> > -                         strcmp(major->id->name, lsm_order_legacy) != 0) {
> > -                             lsm_enabled_set(major, false);
> > +             lsm_for_each_raw(lsm) {
> > +                     if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
> > +                          strcmp(lsm->id->name, lsm_order_legacy)) {
> > +                             lsm_enabled_set(lsm, false);
> >                               init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> > -                                        lsm_order_legacy, major->id->name);
> > +                                        lsm_order_legacy, lsm->id->name);
> >                       }
> >               }
> >       }
> >
> > -     sep = kstrdup(order, GFP_KERNEL);
> > +     /* LSM_ORDER_FIRST */
> > +     lsm_for_each_raw(lsm) {
> > +             if (lsm->order == LSM_ORDER_FIRST)
> > +                     lsm_order_append(lsm, "first");
> > +     }
> > +
> > +     /* Normal or "mutable" LSMs */
>
> Paul, there's a reason for another set of eyes reviewing patches and yes, even,
> comments.

Mimi, if you've read my emails over the years, or discussed this topic
with me in person or off-list, you'll know that I almost always* hold
my patches to the same standard as any other contributor; even in
cases where I know there is likely to be no review beyond my own.

[*] There have been exceptions in the past due to build breakages and
other "critical" fixes that need to be merged ASAP, but even then the
patches are posted and subject to review and additional
patching/fixing afterwards.

>  What are "mutable" LSMs?!

As the "or" would indicate, they are "normal" LSMs where the ordering
is mutable, as opposed to the "first" or "last" LSMs.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v4 17/34] lsm: cleanup initialize_lsm() and rename to lsm_init_single()
From: Mimi Zohar @ 2025-09-18 15:28 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-53-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Rename initialize_lsm() to be more consistent with the rest of the LSM
> initialization changes and rework the function itself to better fit
> with the "exit on fail" coding pattern.
> 
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 24/34] loadpin: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-18 15:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-integrity, selinux, John Johansen,
	Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <598e9ed199ba23e7e11c5ea29132bd2e3202305f.camel@linux.ibm.com>

On Thu, Sep 18, 2025 at 7:16 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > Acked-by: Kees Cook <kees@kernel.org>
> > Reviewed-by: John Johansen <john.johhansen@canonical.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> For the first couple of iterations, the patch descriptions needed to be added or
> improved upon.

As was discussed in the first cover letter, and in the related
reviews, the first iteration was simply a FYI primarily for the sake
of Casey who was working on a patchset which has some overlap.

> Some of the patch descriptions are still missing (e.g. 25, 27,
> etc).  Is this intentional because you feel it is redundant ...

Yes.  Take this particular patch as an example, the conversion to
using the new initcall mechanism for Loadpin is perhaps one of the
more trivial patches one might see in the kernel, the subject line of
"loadpin: move initcalls to the LSM framework" is sufficient to
document the patch as far as I'm concerned.

> FYI, teaching newbies how to break up a patch set is not easy.  This patch set
> is nicely broken up and would be a good example.  However, leaving out the patch
> description would be teaching the wrong thing.

Documentation is a good and important part of the work we do, but
redundant and/or excessive documentation simply for the sake of
satisfying a checkbox is not a good thing IMO.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v4 16/34] lsm: cleanup the LSM blob size code
From: Mimi Zohar @ 2025-09-18 15:14 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Roberto Sassu, Fan Wu, Mickaël Salaün,
	Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
	Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250916220355.252592-52-paul@paul-moore.com>

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Convert the lsm_blob_size fields to unsigned integers as there is no
> current need for them to be negative, change "lsm_set_blob_size()" to
> "lsm_blob_size_update()" to better reflect reality, and perform some
> other minor cleanups to the associated code.
> 
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>


^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Christoph Hellwig @ 2025-09-18 14:38 UTC (permalink / raw)
  To: Marco Elver
  Cc: Christoph Hellwig, Peter Zijlstra, Boqun Feng, Ingo Molnar,
	Will Deacon, David S. Miller, Luc Van Oostenryck,
	Paul E. McKenney, Alexander Potapenko, Arnd Bergmann,
	Bart Van Assche, Bill Wendling, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
	Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <CANpmjNN8Vx5p+0xZAjHA4s6HaGaEdMf_u1c1jiOf=ZKqYYz9Nw@mail.gmail.com>

On Thu, Sep 18, 2025 at 04:30:55PM +0200, Marco Elver wrote:
> Not officially, but I can try to build something to share if you prefer.
> Or a script that automatically pulls and builds clang for you - I have
> this old script I just updated to the above commit:
> https://gist.github.com/melver/fe8a5fd9e43e21fab569ee24fc9c6072
> Does that help?

Just kicked it off, I'll see how long this will take on my laptop.
At least the error checking that tells me about dependencies and work
that needs to be done before starting the build is nice so that I
hopefully don't have to restart the build too often.

^ permalink raw reply

* Re: [PATCH RFC 2/4] landlock: implement memfd detection
From: Mickaël Salaün @ 2025-09-18 14:26 UTC (permalink / raw)
  To: Abhinav Saxena, Paul Moore
  Cc: Fan Wu, Günther Noack, James Morris, Serge E. Hallyn,
	Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, linux-security-module, linux-kernel,
	linux-kselftest, llvm, Daniel Verkamp, Jeff Xu
In-Reply-To: <20250918.kieghied1ahS@digikod.net>

On Thu, Sep 18, 2025 at 04:07:48PM +0200, Mickaël Salaün wrote:
> On Tue, Jul 22, 2025 at 03:56:38PM -0600, Abhinav Saxena wrote:
> > Fan Wu <wufan@kernel.org> writes:
> > 
> > > On Sat, Jul 19, 2025 at 4:13 AM Abhinav Saxena <xandfury@gmail.com> wrote:
> > >>
> > >> Add is_memfd_file() function to reliably detect memfd files by checking
> > >> for “memfd:” prefix in dentry names on shmem-backed files. This
> > >> distinguishes true memfd files from regular shmem files.
> > >>
> > >> Move domain_is_scoped() to domain.c for reuse across subsystems.
> > >> Add comprehensive kunit tests for memfd detection edge cases.
> > >>
> > >> Signed-off-by: Abhinav Saxena <xandfury@gmail.com>
> > >> —
> > >>  security/landlock/domain.c |  67 +++++++++++++++
> > >>  security/landlock/domain.h |   4 +
> > >>  security/landlock/fs.c     | 210 +++++++++++++++++++++++++++++++++++++++++++++
> > >>  security/landlock/task.c   |  67 —————
> > >>  4 files changed, 281 insertions(+), 67 deletions(-)
> > >
> > > …
> > >
> > >>
> > >> +/**
> > >> + * is_memfd_file - Check if file was created via memfd_create()
> > >> + * @file: File to check
> > >> + *
> > >> + * Returns true if @file was created via memfd_create(), false otherwise.
> > >> + *
> > >> + * memfd files are shmem-backed files with “memfd:” prefix in their dentry name.
> > >> + * This is the definitive way to distinguish memfd files from regular shmem
> > >> + * files.
> > >> + */
> > >> +static bool is_memfd_file(struct file *file)
> > >> +{
> > >> +       const struct dentry *dentry;
> > >> +       const unsigned char *name;
> > >> +       size_t name_len;
> > >> +
> > >> +       /* Fast path: basic validation */
> > >> +       if (unlikely(!file))
> > >> +               return false;
> > >> +
> > >> +       /* Must be shmem-backed first - this is the cheapest definitive check */
> > >> +       if (!shmem_file(file))
> > >> +               return false;
> 
> In which case a memfd would not be detected by this function?
> shmem_file_operations or hugetlbfs_file_operations are always set for
> memfd right?
> 
> We could export memfd_get_seals() to stay consistent over time.
> 
> > >> +
> > >> +#ifdef CONFIG_MEMFD_CREATE
> > >> +
> > >> +       /* Validate dentry and get name info */
> > >> +       dentry = file->f_path.dentry;
> > >> +       if (unlikely(!dentry))
> > >> +               return false;
> > >> +
> > >> +       name_len = dentry->d_name.len;
> > >> +       name = dentry->d_name.name;
> > >> +
> > >> +       /* memfd files always have “memfd:” prefix (6 characters) */
> > >> +       if (name_len < 6 || unlikely(!name))
> > >> +               return false;
> > >> +
> > >> +       /* Check for exact “memfd:” prefix */
> > >> +       return memcmp(name, “memfd:”, 6) == 0;
> > >> +#else
> > >> +       return false;
> > >> +#endif
> > >
> > > I was trying to do something similar early this year but didn’t hear
> > > feedback from the linux-mm folks.
> > > <https://lore.kernel.org/linux-security-module/20250129203932.22165-1-wufan@kernel.org/>
> > >
> > > I have considered this approach but didn’t use it. My concern is,
> > > potentially a malicious user can create a file in a shmem fs, e.g.
> > > tmpfs , with the “memfd:” prefix, which can be used to bypass security
> > > policy.
> 
> In the case of Landlock that should not be a security issue but a
> compatibility issue, which is not better.
> 
> > > (Resending this message due to a misconfiguration with my email
> > > client. Apologies for any inconvenience.)
> > >
> > > -Fan
> > 
> > Hi Fan,
> > 
> > Thanks for your comments.
> > 
> > I agree that an LSM hook into memfd_create() would be a much better
> > solution. In the absence of such a function, do you think adding a

Hmm, we cannot use hook_file_alloc_security() to enforce restriction on
memfd creation because the inode mode is set after this call by
memfd.c:alloc_file().  So we should indeed add a new LSM hook.  That
would also improve security checks on file creation for other file
types.

> > `d_unlinked(dentry)` check could serve as an additional verification?
> > 
> > I say things since I *think* that legitimate memfd files are always
> > unlinked while spoofed tmpfs files remain linked. I could be wrong
> > though.
> 
> We should just use the same checks used by the memfd-related
> syscalls/operations to detect such file.
> 
> > 
> > In any case, we can test this approach using kprobes to validate
> > the behavior.
> > 
> > -Abhinav
> 

^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Marco Elver @ 2025-09-18 14:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Jonathan Corbet, Josh Triplett, Justin Stitt,
	Kees Cook, Kentaro Takeda, Lukas Bulwahn, Mark Rutland,
	Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918141511.GA30263@lst.de>

On Thu, 18 Sept 2025 at 16:15, Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Sep 18, 2025 at 03:59:11PM +0200, Marco Elver wrote:
> > A Clang version that supports `-Wthread-safety-pointer` and the new
> > alias-analysis of capability pointers is required (from this version
> > onwards):
> >
> >       https://github.com/llvm/llvm-project/commit/b4c98fcbe1504841203e610c351a3227f36c92a4 [3]
>
> There's no chance to make say x86 pre-built binaries for that available?

Not officially, but I can try to build something to share if you prefer.
Or a script that automatically pulls and builds clang for you - I have
this old script I just updated to the above commit:
https://gist.github.com/melver/fe8a5fd9e43e21fab569ee24fc9c6072
Does that help?

^ permalink raw reply

* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Christoph Hellwig @ 2025-09-18 14:15 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
	Bill Wendling, Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
	Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
	Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <20250918140451.1289454-1-elver@google.com>

On Thu, Sep 18, 2025 at 03:59:11PM +0200, Marco Elver wrote:
> A Clang version that supports `-Wthread-safety-pointer` and the new
> alias-analysis of capability pointers is required (from this version
> onwards):
> 
> 	https://github.com/llvm/llvm-project/commit/b4c98fcbe1504841203e610c351a3227f36c92a4 [3]

There's no chance to make say x86 pre-built binaries for that available?


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox