Add support for Clang's coming "kcfi_salt" attribute, which is designed
to allow for KCFI prototype hashes to be separated[1]. For example,
normally two "void func(void)" functions would have the same KCFI hash,
but if they wanted their indirect calls to be distinguishable by KCFI,
one could add __kcfi_salt("foo").

To test the result, add a corresponding LKDTM test, CFI_FORWARD_SALT.

Link: https://github.com/KSPP/linux/issues/365 [1]
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Bill Wendling <morbo@google.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: <llvm@lists.linux.dev>
---
 include/linux/compiler_attributes.h | 11 +++++++++++
 drivers/misc/lkdtm/cfi.c            | 27 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index c16d4199bf92..eb3769b6a580 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -164,6 +164,17 @@
  */
 #define __gnu_inline                    __attribute__((__gnu_inline__))
 
+/*
+ * Optional: not supported by gcc
+ *
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#kcfi-salt
+ */
+#if __has_attribute(__kcfi_salt__)
+# define __kcfi_salt(STR)		__attribute__((__kcfi_salt__(STR)))
+#else
+# define __kcfi_salt(STR)
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-malloc-function-attribute
  * clang: https://clang.llvm.org/docs/AttributeReference.html#malloc
diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
index 6a33889d0902..11de35d6b4e5 100644
--- a/drivers/misc/lkdtm/cfi.c
+++ b/drivers/misc/lkdtm/cfi.c
@@ -21,6 +21,13 @@ static noinline int lkdtm_increment_int(int *counter)
 	return *counter;
 }
 
+/* Function matching prototype of lkdtm_increment_int but separate salt. */
+static noinline __kcfi_salt("separate prototype hash")
+void lkdtm_increment_void_again(int *counter)
+{
+	(*counter)++;
+}
+
 /* Don't allow the compiler to inline the calls. */
 static noinline void lkdtm_indirect_call(void (*func)(int *))
 {
@@ -46,6 +53,25 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
 	pr_expected_config(CONFIG_CFI_CLANG);
 }
 
+/*
+ * This tries to call an indirect function with a mismatched hash salt.
+ */
+static void lkdtm_CFI_FORWARD_SALT(void)
+{
+	/*
+	 * Matches lkdtm_increment_void()'s and lkdtm_increment_void_again()'s
+	 * prototypes, but they have different hash salts.
+	 */
+	pr_info("Calling matched prototype ...\n");
+	lkdtm_indirect_call(lkdtm_increment_void);
+
+	pr_info("Calling mismatched hash salt ...\n");
+	lkdtm_indirect_call(lkdtm_increment_void_again);
+
+	pr_err("FAIL: survived mismatched salt function call!\n");
+	pr_expected_config(CONFIG_CFI_CLANG);
+}
+
 /*
  * This can stay local to LKDTM, as there should not be a production reason
  * to disable PAC && SCS.
@@ -193,6 +219,7 @@ static void lkdtm_CFI_BACKWARD(void)
 
 static struct crashtype crashtypes[] = {
 	CRASHTYPE(CFI_FORWARD_PROTO),
+	CRASHTYPE(CFI_FORWARD_SALT),
 	CRASHTYPE(CFI_BACKWARD),
 };
 
-- 
2.34.1

Hi Kees,

On Thu, Jul 17, 2025 at 1:54 AM Kees Cook <kees@kernel.org> wrote:
>
> +/*
> + * This tries to call an indirect function with a mismatched hash salt.
> + */
> +static void lkdtm_CFI_FORWARD_SALT(void)
> +{
> +       /*
> +        * Matches lkdtm_increment_void()'s and lkdtm_increment_void_again()'s
> +        * prototypes, but they have different hash salts.
> +        */
> +       pr_info("Calling matched prototype ...\n");
> +       lkdtm_indirect_call(lkdtm_increment_void);
> +
> +       pr_info("Calling mismatched hash salt ...\n");
> +       lkdtm_indirect_call(lkdtm_increment_void_again);
> +
> +       pr_err("FAIL: survived mismatched salt function call!\n");
> +       pr_expected_config(CONFIG_CFI_CLANG);

Should this test also have a "This is probably expected" message if
the compiler doesn't support the __kcfi_salt__ attribute?

Sami

On Thu, Jul 17, 2025 at 10:53 AM Kees Cook <kees@kernel.org> wrote:
>
> Add support for Clang's coming "kcfi_salt" attribute, which is designed
> to allow for KCFI prototype hashes to be separated[1]. For example,
> normally two "void func(void)" functions would have the same KCFI hash,
> but if they wanted their indirect calls to be distinguishable by KCFI,
> one could add __kcfi_salt("foo").

It would be nice to have a quick sentence inline summarizing how it
will be used, e.g. what kind of functions will need to be annotated.

> To test the result, add a corresponding LKDTM test, CFI_FORWARD_SALT.

Sounds good.

> + * clang: https://clang.llvm.org/docs/AttributeReference.html#kcfi-salt

I guess this anchor will eventually work -- I see you asked about it at:

    https://github.com/llvm/llvm-project/pull/141846#discussion_r2209236363

From what I see, this still has to land in LLVM, right? So I guess
there is still time to land this (and the discussion started years
ago), but if you need to take it quickly as a base for some other work
or similar:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

On Thu, Jul 17, 2025 at 11:51 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> It would be nice to have a quick sentence inline summarizing how it
> will be used, e.g. what kind of functions will need to be annotated.

By the way, related to the use case question: would we eventually want
such an attribute for Rust too? If so, happy to help.

Cheers,
Miguel

On Thu, Jul 17, 2025 at 3:08 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Jul 17, 2025 at 11:51 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > It would be nice to have a quick sentence inline summarizing how it
> > will be used, e.g. what kind of functions will need to be annotated.
>
> By the way, related to the use case question: would we eventually want
> such an attribute for Rust too? If so, happy to help.

I would definitely like to see Rust support for this too. Otherwise,
we would have to drop the attribute when Rust is enabled to avoid CFI
type mismatches with C code.

Sami

On Thu, Jul 17, 2025 at 6:48 PM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> I would definitely like to see Rust support for this too. Otherwise,
> we would have to drop the attribute when Rust is enabled to avoid CFI
> type mismatches with C code.

Thanks for confirming -- added then to:

    https://github.com/Rust-for-Linux/linux/issues/354

I will ask upstream when it lands in LLVM.

Cheers,
Miguel