Le 05/09/2025 à 17:45, Fidal Palamparambil a écrit :
From: Fidal palamparambil <rootuserh...@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 <rootuserh...@gmail.com>
Signed-off-by: Fidal palamparambil <rootuserh...@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 (dhowe...@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 (dhowe...@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
