[PATCH] module : fix signature checker pointer arithmetic and bounds check

[PATCH] module : fix signature checker pointer arithmetic and bounds check

[Fidal Palamparambil]

From: Fidal palamparambil <rootuserhere@gmail.com>

This patch fixes :
 - invalid module_param type (bool_enable_only → bool)
 - unsafe pointer arithmetic on void *
 - missing bounds check for sig_len, preventing underflow/OOB
 - export set_module_sig_enforced for consistency

Signed-off-by : Fidal Palamparambil <rootuserhere@gmail.com>
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
 kernel/module/signing.c    |  48 ++++++++------
 kernel/module/signing.orig | 125 +++++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 18 deletions(-)
 create mode 100644 kernel/module/signing.orig

diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..8dda6cd2fd73 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/* Module signature checker
+/*
+ * Module signature checker
  *
  * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
@@ -20,11 +21,11 @@
 #define MODULE_PARAM_PREFIX "module."
 
 static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
-module_param(sig_enforce, bool_enable_only, 0644);
+module_param(sig_enforce, bool, 0644);
 
 /*
- * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
- * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
+ * Export sig_enforce kernel cmdline parameter to allow other subsystems to
+ * rely on that instead of directly on CONFIG_MODULE_SIG_FORCE config.
  */
 bool is_module_sig_enforced(void)
 {
@@ -36,6 +37,7 @@ void set_module_sig_enforced(void)
 {
 	sig_enforce = true;
 }
+EXPORT_SYMBOL(set_module_sig_enforced);
 
 /*
  * Verify the signature on a module.
@@ -45,44 +47,55 @@ int mod_verify_sig(const void *mod, struct load_info *info)
 	struct module_signature ms;
 	size_t sig_len, modlen = info->len;
 	int ret;
+	const unsigned char *data = mod;
 
 	pr_devel("==>%s(,%zu)\n", __func__, modlen);
 
 	if (modlen <= sizeof(ms))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+	memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));
 
 	ret = mod_check_sig(&ms, modlen, "module");
 	if (ret)
 		return ret;
 
 	sig_len = be32_to_cpu(ms.sig_len);
+
+	/* Ensure sig_len is valid to prevent underflow/oob */
+	if (sig_len > modlen - sizeof(ms))
+		return -EBADMSG;
+
 	modlen -= sig_len + sizeof(ms);
 	info->len = modlen;
 
-	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
 				      VERIFY_USE_SECONDARY_KEYRING,
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
 
+/*
+ * Check signature validity of a module during load.
+ */
 int module_sig_check(struct load_info *info, int flags)
 {
 	int err = -ENODATA;
 	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	const char *reason;
-	const void *mod = info->hdr;
+	const unsigned char *mod = info->hdr;
 	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
 				       MODULE_INIT_IGNORE_VERMAGIC);
+
 	/*
-	 * Do not allow mangled modules as a module with version information
-	 * removed is no longer the module that was signed.
+	 * Do not allow mangled modules: a module with version info removed
+	 * is no longer the module that was signed.
 	 */
 	if (!mangled_module &&
 	    info->len > markerlen &&
-	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
-		/* We truncate the module to discard the signature */
+	    memcmp(mod + info->len - markerlen,
+		   MODULE_SIG_STRING, markerlen) == 0) {
+		/* Truncate the module to discard the signature marker */
 		info->len -= markerlen;
 		err = mod_verify_sig(mod, info);
 		if (!err) {
@@ -92,9 +105,8 @@ int module_sig_check(struct load_info *info, int flags)
 	}
 
 	/*
-	 * We don't permit modules to be loaded into the trusted kernels
-	 * without a valid signature on them, but if we're not enforcing,
-	 * certain errors are non-fatal.
+	 * Enforced mode: only allow modules with a valid signature.
+	 * Non-enforced mode: certain errors are downgraded to warnings.
 	 */
 	switch (err) {
 	case -ENODATA:
@@ -106,12 +118,12 @@ int module_sig_check(struct load_info *info, int flags)
 	case -ENOKEY:
 		reason = "module with unavailable key";
 		break;
-
 	default:
 		/*
-		 * All other errors are fatal, including lack of memory,
-		 * unparseable signatures, and signature check failures --
-		 * even if signatures aren't required.
+		 * All other errors are fatal, including:
+		 * - OOM
+		 * - unparseable signatures
+		 * - invalid signature failures
 		 */
 		return err;
 	}
diff --git a/kernel/module/signing.orig b/kernel/module/signing.orig
new file mode 100644
index 000000000000..a2ff4242e623
--- /dev/null
+++ b/kernel/module/signing.orig
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Module signature checker
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/module_signature.h>
+#include <linux/string.h>
+#include <linux/verification.h>
+#include <linux/security.h>
+#include <crypto/public_key.h>
+#include <uapi/linux/module.h>
+#include "internal.h"
+
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "module."
+
+static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
+module_param(sig_enforce, bool_enable_only, 0644);
+
+/*
+ * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
+ * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
+ */
+bool is_module_sig_enforced(void)
+{
+	return sig_enforce;
+}
+EXPORT_SYMBOL(is_module_sig_enforced);
+
+void set_module_sig_enforced(void)
+{
+	sig_enforce = true;
+}
+
+/*
+ * Verify the signature on a module.
+ */
+int mod_verify_sig(const void *mod, struct load_info *info)
+{
+	struct module_signature ms;
+	size_t sig_len, modlen = info->len;
+	int ret;
+
+	pr_devel("==>%s(,%zu)\n", __func__, modlen);
+
+	if (modlen <= sizeof(ms))
+		return -EBADMSG;
+
+	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+
+	ret = mod_check_sig(&ms, modlen, "module");
+	if (ret)
+		return ret;
+
+	sig_len = be32_to_cpu(ms.sig_len);
+	modlen -= sig_len + sizeof(ms);
+	info->len = modlen;
+
+	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+				      VERIFY_USE_SECONDARY_KEYRING,
+				      VERIFYING_MODULE_SIGNATURE,
+				      NULL, NULL);
+}
+
+int module_sig_check(struct load_info *info, int flags)
+{
+	int err = -ENODATA;
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+	const char *reason;
+	const void *mod = info->hdr;
+	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
+				       MODULE_INIT_IGNORE_VERMAGIC);
+	/*
+	 * Do not allow mangled modules as a module with version information
+	 * removed is no longer the module that was signed.
+	 */
+	if (!mangled_module &&
+	    info->len > markerlen &&
+	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
+		/* We truncate the module to discard the signature */
+		info->len -= markerlen;
+		err = mod_verify_sig(mod, info);
+		if (!err) {
+			info->sig_ok = true;
+			return 0;
+		}
+	}
+
+	/*
+	 * We don't permit modules to be loaded into the trusted kernels
+	 * without a valid signature on them, but if we're not enforcing,
+	 * certain errors are non-fatal.
+	 */
+	switch (err) {
+	case -ENODATA:
+		reason = "unsigned module";
+		break;
+	case -ENOPKG:
+		reason = "module with unsupported crypto";
+		break;
+	case -ENOKEY:
+		reason = "module with unavailable key";
+		break;
+
+	default:
+		/*
+		 * All other errors are fatal, including lack of memory,
+		 * unparseable signatures, and signature check failures --
+		 * even if signatures aren't required.
+		 */
+		return err;
+	}
+
+	if (is_module_sig_enforced()) {
+		pr_notice("Loading of %s is rejected\n", reason);
+		return -EKEYREJECTED;
+	}
+
+	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+}
-- 
2.50.1.windows.1

Re: [PATCH] module : fix signature checker pointer arithmetic and bounds check

[Luis Chamberlain]

On Fri, Sep 05, 2025 at 07:45:49PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
> 
> This patch fixes :
>  - invalid module_param type (bool_enable_only → bool)
>  - unsafe pointer arithmetic on void *
>  - missing bounds check for sig_len, preventing underflow/OOB
>  - export set_module_sig_enforced for consistency
> 
> Signed-off-by : Fidal Palamparambil <rootuserhere@gmail.com>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>

I will ask that other maintainer ignore your patches moving forward.
Clearly this is garbage gen AI crap code. The list, the double SOB,
and your clear wreckless post is a good example to just never want
to apply any patch from you.

  Luis

Re: [PATCH] module : fix signature checker pointer arithmetic and bounds check

[Christophe Leroy]

Le 05/09/2025 à 17:45, Fidal Palamparambil a écrit :
> From: Fidal palamparambil <rootuserhere@gmail.com>
> 
> This patch fixes :
>   - invalid module_param type (bool_enable_only → bool)

Can you explain what the problem is ? Why do you say bool_enable_only is 
invalid ? Was generalised by commit d19f05d8a8fa ("kernel/params.c: 
generalize bool_enable_only")

>   - unsafe pointer arithmetic on void *

Why is it unsafe in Linux Kernel ? See https://lkml.org/lkml/2022/2/24/978

>   - missing bounds check for sig_len, preventing underflow/OOB

This is checked by mod_check_sig(), why check it again ?

>   - export set_module_sig_enforced for consistency

Consistency with what ? Can you tell which module needs it ?

> 
> Signed-off-by : Fidal Palamparambil <rootuserhere@gmail.com>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>

Why a double sob ?

> ---
>   kernel/module/signing.c    |  48 ++++++++------
>   kernel/module/signing.orig | 125 +++++++++++++++++++++++++++++++++++++

Why adding this .orig file into the kernel at all ?

>   2 files changed, 155 insertions(+), 18 deletions(-)
>   create mode 100644 kernel/module/signing.orig
> 
> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index a2ff4242e623..8dda6cd2fd73 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c
> @@ -1,5 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
> -/* Module signature checker
> +/*
> + * Module signature checker

Don't mix cosmetic changes and real changes, you are making 
bisectability more difficult.

>    *
>    * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
>    * Written by David Howells (dhowells@redhat.com)
> @@ -20,11 +21,11 @@
>   #define MODULE_PARAM_PREFIX "module."
> 
>   static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> -module_param(sig_enforce, bool_enable_only, 0644);
> +module_param(sig_enforce, bool, 0644);
> 
>   /*
> - * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> - * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
> + * Export sig_enforce kernel cmdline parameter to allow other subsystems to
> + * rely on that instead of directly on CONFIG_MODULE_SIG_FORCE config.
>    */
>   bool is_module_sig_enforced(void)
>   {
> @@ -36,6 +37,7 @@ void set_module_sig_enforced(void)
>   {
>          sig_enforce = true;
>   }
> +EXPORT_SYMBOL(set_module_sig_enforced);
> 
>   /*
>    * Verify the signature on a module.
> @@ -45,44 +47,55 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>          struct module_signature ms;
>          size_t sig_len, modlen = info->len;
>          int ret;
> +       const unsigned char *data = mod;

Pointless change.

> 
>          pr_devel("==>%s(,%zu)\n", __func__, modlen);
> 
>          if (modlen <= sizeof(ms))
>                  return -EBADMSG;
> 
> -       memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> +       memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));

Pointless change

> 
>          ret = mod_check_sig(&ms, modlen, "module");
>          if (ret)
>                  return ret;
> 
>          sig_len = be32_to_cpu(ms.sig_len);
> +
> +       /* Ensure sig_len is valid to prevent underflow/oob */
> +       if (sig_len > modlen - sizeof(ms))
> +               return -EBADMSG;

Already verified by mod_check_sig()

> +
>          modlen -= sig_len + sizeof(ms);
>          info->len = modlen;
> 
> -       return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
> +       return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,

pointless change

>                                        VERIFY_USE_SECONDARY_KEYRING,
>                                        VERIFYING_MODULE_SIGNATURE,
>                                        NULL, NULL);
>   }
> 
> +/*
> + * Check signature validity of a module during load.
> + */
>   int module_sig_check(struct load_info *info, int flags)
>   {
>          int err = -ENODATA;
>          const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
>          const char *reason;
> -       const void *mod = info->hdr;
> +       const unsigned char *mod = info->hdr;

info->hdr is not void*, how can this work without a cast ?

>          bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
>                                         MODULE_INIT_IGNORE_VERMAGIC);
> +

Unrelated cosmetic change

>          /*
> -        * Do not allow mangled modules as a module with version information
> -        * removed is no longer the module that was signed.
> +        * Do not allow mangled modules: a module with version info removed
> +        * is no longer the module that was signed.
>           */
>          if (!mangled_module &&
>              info->len > markerlen &&
> -           memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> -               /* We truncate the module to discard the signature */
> +           memcmp(mod + info->len - markerlen,
> +                  MODULE_SIG_STRING, markerlen) == 0) {
> +               /* Truncate the module to discard the signature marker */

Cosmetic and pointless change.

>                  info->len -= markerlen;
>                  err = mod_verify_sig(mod, info);
>                  if (!err) {
> @@ -92,9 +105,8 @@ int module_sig_check(struct load_info *info, int flags)
>          }
> 
>          /*
> -        * We don't permit modules to be loaded into the trusted kernels
> -        * without a valid signature on them, but if we're not enforcing,
> -        * certain errors are non-fatal.
> +        * Enforced mode: only allow modules with a valid signature.
> +        * Non-enforced mode: certain errors are downgraded to warnings.
>           */
>          switch (err) {
>          case -ENODATA:
> @@ -106,12 +118,12 @@ int module_sig_check(struct load_info *info, int flags)
>          case -ENOKEY:
>                  reason = "module with unavailable key";
>                  break;
> -

Cosmetic

>          default:
>                  /*
> -                * All other errors are fatal, including lack of memory,
> -                * unparseable signatures, and signature check failures --
> -                * even if signatures aren't required.
> +                * All other errors are fatal, including:
> +                * - OOM
> +                * - unparseable signatures
> +                * - invalid signature failures
>                   */
>                  return err;
>          }
> diff --git a/kernel/module/signing.orig b/kernel/module/signing.orig
> new file mode 100644
> index 000000000000..a2ff4242e623
> --- /dev/null
> +++ b/kernel/module/signing.orig
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Module signature checker
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/module_signature.h>
> +#include <linux/string.h>
> +#include <linux/verification.h>
> +#include <linux/security.h>
> +#include <crypto/public_key.h>
> +#include <uapi/linux/module.h>
> +#include "internal.h"
> +
> +#undef MODULE_PARAM_PREFIX
> +#define MODULE_PARAM_PREFIX "module."
> +
> +static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> +module_param(sig_enforce, bool_enable_only, 0644);
> +
> +/*
> + * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> + * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
> + */
> +bool is_module_sig_enforced(void)
> +{
> +       return sig_enforce;
> +}
> +EXPORT_SYMBOL(is_module_sig_enforced);
> +
> +void set_module_sig_enforced(void)
> +{
> +       sig_enforce = true;
> +}
> +
> +/*
> + * Verify the signature on a module.
> + */
> +int mod_verify_sig(const void *mod, struct load_info *info)
> +{
> +       struct module_signature ms;
> +       size_t sig_len, modlen = info->len;
> +       int ret;
> +
> +       pr_devel("==>%s(,%zu)\n", __func__, modlen);
> +
> +       if (modlen <= sizeof(ms))
> +               return -EBADMSG;
> +
> +       memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> +
> +       ret = mod_check_sig(&ms, modlen, "module");
> +       if (ret)
> +               return ret;
> +
> +       sig_len = be32_to_cpu(ms.sig_len);
> +       modlen -= sig_len + sizeof(ms);
> +       info->len = modlen;
> +
> +       return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
> +                                     VERIFY_USE_SECONDARY_KEYRING,
> +                                     VERIFYING_MODULE_SIGNATURE,
> +                                     NULL, NULL);
> +}
> +
> +int module_sig_check(struct load_info *info, int flags)
> +{
> +       int err = -ENODATA;
> +       const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> +       const char *reason;
> +       const void *mod = info->hdr;
> +       bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> +                                      MODULE_INIT_IGNORE_VERMAGIC);
> +       /*
> +        * Do not allow mangled modules as a module with version information
> +        * removed is no longer the module that was signed.
> +        */
> +       if (!mangled_module &&
> +           info->len > markerlen &&
> +           memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> +               /* We truncate the module to discard the signature */
> +               info->len -= markerlen;
> +               err = mod_verify_sig(mod, info);
> +               if (!err) {
> +                       info->sig_ok = true;
> +                       return 0;
> +               }
> +       }
> +
> +       /*
> +        * We don't permit modules to be loaded into the trusted kernels
> +        * without a valid signature on them, but if we're not enforcing,
> +        * certain errors are non-fatal.
> +        */
> +       switch (err) {
> +       case -ENODATA:
> +               reason = "unsigned module";
> +               break;
> +       case -ENOPKG:
> +               reason = "module with unsupported crypto";
> +               break;
> +       case -ENOKEY:
> +               reason = "module with unavailable key";
> +               break;
> +
> +       default:
> +               /*
> +                * All other errors are fatal, including lack of memory,
> +                * unparseable signatures, and signature check failures --
> +                * even if signatures aren't required.
> +                */
> +               return err;
> +       }
> +
> +       if (is_module_sig_enforced()) {
> +               pr_notice("Loading of %s is rejected\n", reason);
> +               return -EKEYREJECTED;
> +       }
> +
> +       return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> +}
> --
> 2.50.1.windows.1
> 
> 

Re: [PATCH] module : fix signature checker pointer arithmetic and bounds check

[Luis Chamberlain]

On Tue, Sep 09, 2025 at 11:32:27AM +0200, Christophe Leroy wrote:
> 
> 
> Le 05/09/2025 à 17:45, Fidal Palamparambil a écrit :
> > From: Fidal palamparambil <rootuserhere@gmail.com>
> > 
> > This patch fixes :
> >   - invalid module_param type (bool_enable_only → bool)
> 
> Can you explain what the problem is ? Why do you say bool_enable_only is
> invalid ? Was generalised by commit d19f05d8a8fa ("kernel/params.c:
> generalize bool_enable_only")

Christophe, I will try to save you time. You can ignore the submitter's patch.
They are a troll. I recommend you add them to your ignore list.

  Luis