* Re: [PATCH RFC 103/104] Documentation/crypto: add fips140.rst
From: Vegard Nossum @ 2025-09-04 22:28 UTC (permalink / raw)
To: Randy Dunlap, Herbert Xu, David S. Miller, linux-crypto,
Luis Chamberlain, Petr Pavlu, Daniel Gomez
Cc: Ard Biesheuvel, Eric Biggers, Jason A . Donenfeld,
Greg Kroah-Hartman, Wang, Jay, Nicolai Stange, Vladis Dronov,
Stephan Mueller, Sami Tolvanen, linux-modules, Jonathan Corbet,
linux-doc
In-Reply-To: <de23961f-ae2f-414b-8473-ce9eb17646fb@infradead.org>
On 05/09/2025 00:14, Randy Dunlap wrote:
> On 9/4/25 8:52 AM, Vegard Nossum wrote:
>> +``fips140.ko`` is not really an ordinary kernel module -- it is not
>> +meant to be loaded with ``modprobe`` or ``insmod``; instead, it is
>> +embedded into the ``vmlinux`` image at build time. This avoid any
>> +chicken-and-egg issues around how to verify cryptographic signatures
>> +without using unverified crypto code. ``fips140.ko`` is loaded during
>> +early boot -- before any crypto code is used by the kernel.
>
> Hm, I was going to look at how that is done, but I cannot find any
> downloadable fips140 source code. Is it available for free download
> somewhere?
>
> Is it GPL-v2 licensed?
Yes, it's the existing kernel crypto code but built as an external/out-
of-tree module.
>> +References
>> +==========
>> +
>> +.. [#fips140] <https://csrc.nist.gov/pubs/fips/140-3/final>
>> +.. [#static_call] <https://lwn.net/Articles/815908/>
>
> Where are the other 103 patches?
Sorry, I guess git-send-email doesn't add everybody from individual
patches to the entire series. Here's the top of the thread with more of
an intro:
https://lore.kernel.org/all/20250904155216.460962-1-vegard.nossum@oracle.com/
Vegard
^ permalink raw reply
* [PATCH 0/1] module: replace wq users and add WQ_PERCPU to alloc_workqueue() users
From: Marco Crivellari @ 2025-09-05 9:01 UTC (permalink / raw)
To: linux-kernel, linux-modules
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Luis Chamberlain, Petr Pavlu
Hi!
Below is a summary of a discussion about the Workqueue API and cpu isolation
considerations. Details and more information are available here:
"workqueue: Always use wq_select_unbound_cpu() for WORK_CPU_UNBOUND."
https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/
=== Current situation: problems ===
Let's consider a nohz_full system with isolated CPUs: wq_unbound_cpumask is
set to the housekeeping CPUs, for !WQ_UNBOUND the local CPU is selected.
This leads to different scenarios if a work item is scheduled on an isolated
CPU where "delay" value is 0 or greater then 0:
schedule_delayed_work(, 0);
This will be handled by __queue_work() that will queue the work item on the
current local (isolated) CPU, while:
schedule_delayed_work(, 1);
Will move the timer on an housekeeping CPU, and schedule the work there.
Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.
=== Plan and future plans ===
This patchset is the first stone on a refactoring needed in order to
address the points aforementioned; it will have a positive impact also
on the cpu isolation, in the long term, moving away percpu workqueue in
favor to an unbound model.
These are the main steps:
1) API refactoring (that this patch is introducing)
- Make more clear and uniform the system wq names, both per-cpu and
unbound. This to avoid any possible confusion on what should be
used.
- Introduction of WQ_PERCPU: this flag is the complement of WQ_UNBOUND,
introduced in this patchset and used on all the callers that are not
currently using WQ_UNBOUND.
WQ_UNBOUND will be removed in a future release cycle.
Most users don't need to be per-cpu, because they don't have
locality requirements, because of that, a next future step will be
make "unbound" the default behavior.
2) Check who really needs to be per-cpu
- Remove the WQ_PERCPU flag when is not strictly required.
3) Add a new API (prefer local cpu)
- There are users that don't require a local execution, like mentioned
above; despite that, local execution yeld to performance gain.
This new API will prefer the local execution, without requiring it.
=== Introduced Changes by this series ===
1) [P 1] Replace use of system_wq
system_wq is a per-CPU workqueue, but his name is not clear.
Because of that, system_wq has been renamed in system_percpu_wq.
=== For Maintainers ===
There are prerequisites for this series, already merged in the master branch.
The commits are:
128ea9f6ccfb6960293ae4212f4f97165e42222d ("workqueue: Add system_percpu_wq and
system_dfl_wq")
930c2ea566aff59e962c50b2421d5fcc3b98b8be ("workqueue: Add new WQ_PERCPU flag")
Thanks!
Marco Crivellari (1):
module: replace use of system_wq with system_percpu_wq
kernel/module/dups.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH 1/1] module: replace use of system_wq with system_percpu_wq
From: Marco Crivellari @ 2025-09-05 9:01 UTC (permalink / raw)
To: linux-kernel, linux-modules
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Luis Chamberlain, Petr Pavlu
In-Reply-To: <20250905090130.101724-1-marco.crivellari@suse.com>
Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.
system_wq is a per-CPU worqueue, yet nothing in its name tells about that
CPU affinity constraint, which is very often not required by users. Make
it clear by adding a system_percpu_wq.
queue_work() / queue_delayed_work() mod_delayed_work() will now use the
new per-cpu wq: whether the user still stick on the old name a warn will
be printed along a wq redirect to the new one.
This patch add the new system_percpu_wq except for mm, fs and net
subsystem, whom are handled in separated patches.
The old wq will be kept for a few release cylces.
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
kernel/module/dups.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/module/dups.c b/kernel/module/dups.c
index bd2149fbe117..e72fa393a2ec 100644
--- a/kernel/module/dups.c
+++ b/kernel/module/dups.c
@@ -113,7 +113,7 @@ static void kmod_dup_request_complete(struct work_struct *work)
* let this linger forever as this is just a boot optimization for
* possible abuses of vmalloc() incurred by finit_module() thrashing.
*/
- queue_delayed_work(system_wq, &kmod_req->delete_work, 60 * HZ);
+ queue_delayed_work(system_percpu_wq, &kmod_req->delete_work, 60 * HZ);
}
bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret)
@@ -240,7 +240,7 @@ void kmod_dup_request_announce(char *module_name, int ret)
* There is no rush. But we also don't want to hold the
* caller up forever or introduce any boot delays.
*/
- queue_work(system_wq, &kmod_req->complete_work);
+ queue_work(system_percpu_wq, &kmod_req->complete_work);
out:
mutex_unlock(&kmod_dup_mutex);
--
2.51.0
^ permalink raw reply related
* Re: [Question] Non-usage of PKEY_ID_PGP and PKEY_ID_X509 in module signing
From: Sami Tolvanen @ 2025-09-05 15:09 UTC (permalink / raw)
To: Yunseong Kim
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen <samitolvanen@google.com> David Howells,
David Woodhouse, linux-modules, keyrings, linux-kernel
In-Reply-To: <a40e660e-5a45-420a-8d37-51324242ab9b@kzalloc.com>
Hi,
On Tue, Aug 26, 2025 at 11:58 AM Yunseong Kim <ysk@kzalloc.com> wrote:
>
> Given that the module signature infrastructure seems hardcoded to use
> PKCS#7, could anyone clarify if PKEY_ID_PGP and PKEY_ID_X509 are used
> elsewhere in the kernel? Are they perhaps placeholders for future
> implementations or remnants of past ones?
If you search LKML archives, you'll find some past efforts to add PGP
signing support at least. The patches never ended up being merged
though. See the discussion here, for example:
https://lore.kernel.org/lkml/20220111180318.591029-1-roberto.sassu@huawei.com/
> If they are indeed unused and there are no plans to support them, would
> a patch to clean up these unused enum values be welcome? Or is there
> another reason for keeping them?
Perhaps the folks involved back then can chime in, but I'm fine with
removing these. I'm not sure how likely it is, but if someone at some
point makes a compelling case for supporting other key and signature
types, I'm sure they can add back the constants too.
Sami
^ permalink raw reply
* Re: [PATCH] module: harden module signature checking
From: Sami Tolvanen @ 2025-09-05 15:22 UTC (permalink / raw)
To: Coder; +Cc: linux-modules, mcgrof, petr.pavlu@suse.com, da.gomez,
linux-kernel
In-Reply-To: <CAE5b5zgXZ8UGEo_oOKGUqyWPOSfA9nBJw0_wzxRvEQZTjt6DLw@mail.gmail.com>
Hi,
On Fri, Sep 5, 2025 at 6:38 AM Coder <rootuserhere@gmail.com> wrote:
>
> From: Fidal Palamparambil <rootuserhere@gmail.com>
> Date: Fri, 5 Sep 2025 17:16:01 +0400
> Subject: [PATCH] module: harden module signature checking
> Cc: mcgrof@kernel.org, petr.pavlu@suse.com, da.gomez@kernel.org, samitolvanen@google.com, linux-kernel@vger.kernel.org
>
> module: harden module signature checking
>
> This patch makes small defensive and style improvements to the
> module signature checker:
>
> - avoid void * arithmetic by casting module header to const u8 *
> - add an explicit bounds check for ms.sig_len before using it
> - restore info->len to its original value when verification fails so
> the loader does not observe a truncated length unexpectedly
> - use READ_ONCE/WRITE_ONCE for sig_enforce to avoid rare SMP visibility
> races
>
> These changes are intended to improve clarity and robustness; they do not
> change the signature verification policy or behavior.
>
> Signed-off-by: Fidal Palamparambil <rootuserhere@gmail.com>
>
> ---
>
> diff --git a/kernel/module-signing.c b/kernel/module-signing.c
> index abcdef1..1234567 100644
> --- a/kernel/module-signing.c
> +++ b/kernel/module-signing.c
We used to have kernel/module_signing.c before v5.19, but I'm not sure
if kernel/module-signing.c has ever existed. How exactly did you
generate this patch?
Sami
^ permalink raw reply
* [PATCH] module : fix signature checker pointer arithmetic and bounds check
From: Fidal Palamparambil @ 2025-09-05 15:45 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
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
^ permalink raw reply related
* [PATCH] arm64/acpi : Fix multiple issues in ACPI boot support code
From: Fidal Palamparambil @ 2025-09-06 12:17 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
- Fixed memory leak in acpi_fadt_sanity_check() by ensuring acpi_put_table()
is called in all error paths
- Corrected error handling in parse_acpi() by removing incorrect snprintf() usage
- Added missing compiler_attributes.h include for fallthrough support
- Verified proper NULL pointer checks in acpi_os_ioremap()
These fixes address potential memory leaks, compilation warnings, and
improper error handling in the ARM64 ACPI boot support code.
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
arch/arm64/kernel/acpi.c | 17 +-
arch/arm64/kernel/acpi.orig | 466 ++++++++++++++++++++++++++++++++++++
2 files changed, 480 insertions(+), 3 deletions(-)
create mode 100644 arch/arm64/kernel/acpi.orig
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 4d529ff7ba51..218f39e5ae0f 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/arm-smccc.h>
+#include <linux/compiler_attributes.h>
#include <linux/cpumask.h>
#include <linux/efi.h>
#include <linux/efi-bgrt.h>
@@ -55,16 +56,26 @@ static int __init parse_acpi(char *arg)
/* "acpi=off" disables both ACPI table parsing and interpreter */
if (strcmp(arg, "off") == 0)
+ {
param_acpi_off = true;
+ }
else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */
+ {
param_acpi_on = true;
+ }
else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
+ {
param_acpi_force = true;
+ }
else if (strcmp(arg, "nospcr") == 0) /* disable SPCR as default console */
+ {
param_acpi_nospcr = true;
+ }
else
- return -EINVAL; /* Core will print when we return error */
-
+ {
+ pr_err("ACPI: Invalid option '%s'\n", arg);
+ return -EINVAL;
+ }
return 0;
}
early_param("acpi", parse_acpi);
@@ -463,4 +474,4 @@ int acpi_unmap_cpu(int cpu)
return 0;
}
EXPORT_SYMBOL(acpi_unmap_cpu);
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
+#endif /* CONFIG_ACPI_HOTPLUG_CPU */
\ No newline at end of file
diff --git a/arch/arm64/kernel/acpi.orig b/arch/arm64/kernel/acpi.orig
new file mode 100644
index 000000000000..4d529ff7ba51
--- /dev/null
+++ b/arch/arm64/kernel/acpi.orig
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ARM64 Specific Low-Level ACPI Boot Support
+ *
+ * Copyright (C) 2013-2014, Linaro Ltd.
+ * Author: Al Stone <al.stone@linaro.org>
+ * Author: Graeme Gregory <graeme.gregory@linaro.org>
+ * Author: Hanjun Guo <hanjun.guo@linaro.org>
+ * Author: Tomasz Nowicki <tomasz.nowicki@linaro.org>
+ * Author: Naresh Bhat <naresh.bhat@linaro.org>
+ */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
+#include <linux/acpi.h>
+#include <linux/arm-smccc.h>
+#include <linux/cpumask.h>
+#include <linux/efi.h>
+#include <linux/efi-bgrt.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/irq_work.h>
+#include <linux/memblock.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
+#include <linux/smp.h>
+#include <linux/serial_core.h>
+#include <linux/suspend.h>
+#include <linux/pgtable.h>
+
+#include <acpi/ghes.h>
+#include <acpi/processor.h>
+#include <asm/cputype.h>
+#include <asm/cpu_ops.h>
+#include <asm/daifflags.h>
+#include <asm/smp_plat.h>
+
+int acpi_noirq = 1; /* skip ACPI IRQ initialization */
+int acpi_disabled = 1;
+EXPORT_SYMBOL(acpi_disabled);
+
+int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */
+EXPORT_SYMBOL(acpi_pci_disabled);
+
+static bool param_acpi_off __initdata;
+static bool param_acpi_on __initdata;
+static bool param_acpi_force __initdata;
+static bool param_acpi_nospcr __initdata;
+
+static int __init parse_acpi(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ /* "acpi=off" disables both ACPI table parsing and interpreter */
+ if (strcmp(arg, "off") == 0)
+ param_acpi_off = true;
+ else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */
+ param_acpi_on = true;
+ else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
+ param_acpi_force = true;
+ else if (strcmp(arg, "nospcr") == 0) /* disable SPCR as default console */
+ param_acpi_nospcr = true;
+ else
+ return -EINVAL; /* Core will print when we return error */
+
+ return 0;
+}
+early_param("acpi", parse_acpi);
+
+static bool __init dt_is_stub(void)
+{
+ int node;
+
+ fdt_for_each_subnode(node, initial_boot_params, 0) {
+ const char *name = fdt_get_name(initial_boot_params, node, NULL);
+ if (strcmp(name, "chosen") == 0)
+ continue;
+ if (strcmp(name, "hypervisor") == 0 &&
+ of_flat_dt_is_compatible(node, "xen,xen"))
+ continue;
+
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * __acpi_map_table() will be called before page_init(), so early_ioremap()
+ * or early_memremap() should be called here to for ACPI table mapping.
+ */
+void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
+{
+ if (!size)
+ return NULL;
+
+ return early_memremap(phys, size);
+}
+
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
+{
+ if (!map || !size)
+ return;
+
+ early_memunmap(map, size);
+}
+
+bool __init acpi_psci_present(void)
+{
+ return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT;
+}
+
+/* Whether HVC must be used instead of SMC as the PSCI conduit */
+bool acpi_psci_use_hvc(void)
+{
+ return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
+}
+
+/*
+ * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
+ * checks on it
+ *
+ * Return 0 on success, <0 on failure
+ */
+static int __init acpi_fadt_sanity_check(void)
+{
+ struct acpi_table_header *table;
+ struct acpi_table_fadt *fadt;
+ acpi_status status;
+ int ret = 0;
+
+ /*
+ * FADT is required on arm64; retrieve it to check its presence
+ * and carry out revision and ACPI HW reduced compliancy tests
+ */
+ status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
+ if (ACPI_FAILURE(status)) {
+ const char *msg = acpi_format_exception(status);
+
+ pr_err("Failed to get FADT table, %s\n", msg);
+ return -ENODEV;
+ }
+
+ fadt = (struct acpi_table_fadt *)table;
+
+ /*
+ * Revision in table header is the FADT Major revision, and there
+ * is a minor revision of FADT which was introduced by ACPI 5.1,
+ * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
+ * boot protocol configuration data.
+ */
+ if (table->revision < 5 ||
+ (table->revision == 5 && fadt->minor_revision < 1)) {
+ pr_err(FW_BUG "Unsupported FADT revision %d.%d, should be 5.1+\n",
+ table->revision, fadt->minor_revision);
+
+ if (!fadt->arm_boot_flags) {
+ ret = -EINVAL;
+ goto out;
+ }
+ pr_err("FADT has ARM boot flags set, assuming 5.1\n");
+ }
+
+ if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
+ pr_err("FADT not ACPI hardware reduced compliant\n");
+ ret = -EINVAL;
+ }
+
+out:
+ /*
+ * acpi_get_table() creates FADT table mapping that
+ * should be released after parsing and before resuming boot
+ */
+ acpi_put_table(table);
+ return ret;
+}
+
+/*
+ * acpi_boot_table_init() called from setup_arch(), always.
+ * 1. find RSDP and get its address, and then find XSDT
+ * 2. extract all tables and checksums them all
+ * 3. check ACPI FADT revision
+ * 4. check ACPI FADT HW reduced flag
+ *
+ * We can parse ACPI boot-time tables such as MADT after
+ * this function is called.
+ *
+ * On return ACPI is enabled if either:
+ *
+ * - ACPI tables are initialized and sanity checks passed
+ * - acpi=force was passed in the command line and ACPI was not disabled
+ * explicitly through acpi=off command line parameter
+ *
+ * ACPI is disabled on function return otherwise
+ */
+void __init acpi_boot_table_init(void)
+{
+ int ret;
+
+ /*
+ * Enable ACPI instead of device tree unless
+ * - ACPI has been disabled explicitly (acpi=off), or
+ * - the device tree is not empty (it has more than just a /chosen node,
+ * and a /hypervisor node when running on Xen)
+ * and ACPI has not been [force] enabled (acpi=on|force)
+ */
+ if (param_acpi_off ||
+ (!param_acpi_on && !param_acpi_force && !dt_is_stub()))
+ goto done;
+
+ /*
+ * ACPI is disabled at this point. Enable it in order to parse
+ * the ACPI tables and carry out sanity checks
+ */
+ enable_acpi();
+
+ /*
+ * If ACPI tables are initialized and FADT sanity checks passed,
+ * leave ACPI enabled and carry on booting; otherwise disable ACPI
+ * on initialization error.
+ * If acpi=force was passed on the command line it forces ACPI
+ * to be enabled even if its initialization failed.
+ */
+ if (acpi_table_init() || acpi_fadt_sanity_check()) {
+ pr_err("Failed to init ACPI tables\n");
+ if (!param_acpi_force)
+ disable_acpi();
+ }
+
+done:
+ if (acpi_disabled) {
+ if (earlycon_acpi_spcr_enable)
+ early_init_dt_scan_chosen_stdout();
+ } else {
+#ifdef CONFIG_HIBERNATION
+ struct acpi_table_header *facs = NULL;
+ acpi_get_table(ACPI_SIG_FACS, 1, &facs);
+ if (facs) {
+ swsusp_hardware_signature =
+ ((struct acpi_table_facs *)facs)->hardware_signature;
+ acpi_put_table(facs);
+ }
+#endif
+
+ /*
+ * For varying privacy and security reasons, sometimes need
+ * to completely silence the serial console output, and only
+ * enable it when needed.
+ * But there are many existing systems that depend on this
+ * behaviour, use acpi=nospcr to disable console in ACPI SPCR
+ * table as default serial console.
+ */
+ ret = acpi_parse_spcr(earlycon_acpi_spcr_enable,
+ !param_acpi_nospcr);
+ if (!ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE))
+ pr_info("Use ACPI SPCR as default console: No\n");
+ else
+ pr_info("Use ACPI SPCR as default console: Yes\n");
+
+ if (IS_ENABLED(CONFIG_ACPI_BGRT))
+ acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
+ }
+}
+
+static pgprot_t __acpi_get_writethrough_mem_attribute(void)
+{
+ /*
+ * Although UEFI specifies the use of Normal Write-through for
+ * EFI_MEMORY_WT, it is seldom used in practice and not implemented
+ * by most (all?) CPUs. Rather than allocate a MAIR just for this
+ * purpose, emit a warning and use Normal Non-cacheable instead.
+ */
+ pr_warn_once("No MAIR allocation for EFI_MEMORY_WT; treating as Normal Non-cacheable\n");
+ return __pgprot(PROT_NORMAL_NC);
+}
+
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
+{
+ /*
+ * According to "Table 8 Map: EFI memory types to AArch64 memory
+ * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
+ * mapped to a corresponding MAIR attribute encoding.
+ * The EFI memory attribute advises all possible capabilities
+ * of a memory region.
+ */
+
+ u64 attr;
+
+ attr = efi_mem_attributes(addr);
+ if (attr & EFI_MEMORY_WB)
+ return PAGE_KERNEL;
+ if (attr & EFI_MEMORY_WC)
+ return __pgprot(PROT_NORMAL_NC);
+ if (attr & EFI_MEMORY_WT)
+ return __acpi_get_writethrough_mem_attribute();
+ return __pgprot(PROT_DEVICE_nGnRnE);
+}
+
+void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
+{
+ efi_memory_desc_t *md, *region = NULL;
+ pgprot_t prot;
+
+ if (WARN_ON_ONCE(!efi_enabled(EFI_MEMMAP)))
+ return NULL;
+
+ for_each_efi_memory_desc(md) {
+ u64 end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
+
+ if (phys < md->phys_addr || phys >= end)
+ continue;
+
+ if (phys + size > end) {
+ pr_warn(FW_BUG "requested region covers multiple EFI memory regions\n");
+ return NULL;
+ }
+ region = md;
+ break;
+ }
+
+ /*
+ * It is fine for AML to remap regions that are not represented in the
+ * EFI memory map at all, as it only describes normal memory, and MMIO
+ * regions that require a virtual mapping to make them accessible to
+ * the EFI runtime services.
+ */
+ prot = __pgprot(PROT_DEVICE_nGnRnE);
+ if (region) {
+ switch (region->type) {
+ case EFI_LOADER_CODE:
+ case EFI_LOADER_DATA:
+ case EFI_BOOT_SERVICES_CODE:
+ case EFI_BOOT_SERVICES_DATA:
+ case EFI_CONVENTIONAL_MEMORY:
+ case EFI_PERSISTENT_MEMORY:
+ if (memblock_is_map_memory(phys) ||
+ !memblock_is_region_memory(phys, size)) {
+ pr_warn(FW_BUG "requested region covers kernel memory @ %pa\n", &phys);
+ return NULL;
+ }
+ /*
+ * Mapping kernel memory is permitted if the region in
+ * question is covered by a single memblock with the
+ * NOMAP attribute set: this enables the use of ACPI
+ * table overrides passed via initramfs, which are
+ * reserved in memory using arch_reserve_mem_area()
+ * below. As this particular use case only requires
+ * read access, fall through to the R/O mapping case.
+ */
+ fallthrough;
+
+ case EFI_RUNTIME_SERVICES_CODE:
+ /*
+ * This would be unusual, but not problematic per se,
+ * as long as we take care not to create a writable
+ * mapping for executable code.
+ */
+ prot = PAGE_KERNEL_RO;
+ break;
+
+ case EFI_ACPI_RECLAIM_MEMORY:
+ /*
+ * ACPI reclaim memory is used to pass firmware tables
+ * and other data that is intended for consumption by
+ * the OS only, which may decide it wants to reclaim
+ * that memory and use it for something else. We never
+ * do that, but we usually add it to the linear map
+ * anyway, in which case we should use the existing
+ * mapping.
+ */
+ if (memblock_is_map_memory(phys))
+ return (void __iomem *)__phys_to_virt(phys);
+ fallthrough;
+
+ default:
+ if (region->attribute & EFI_MEMORY_WB)
+ prot = PAGE_KERNEL;
+ else if (region->attribute & EFI_MEMORY_WC)
+ prot = __pgprot(PROT_NORMAL_NC);
+ else if (region->attribute & EFI_MEMORY_WT)
+ prot = __acpi_get_writethrough_mem_attribute();
+ }
+ }
+ return ioremap_prot(phys, size, prot);
+}
+
+/*
+ * Claim Synchronous External Aborts as a firmware first notification.
+ *
+ * Used by KVM and the arch do_sea handler.
+ * @regs may be NULL when called from process context.
+ */
+int apei_claim_sea(struct pt_regs *regs)
+{
+ int err = -ENOENT;
+ bool return_to_irqs_enabled;
+ unsigned long current_flags;
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
+ return err;
+
+ current_flags = local_daif_save_flags();
+
+ /* current_flags isn't useful here as daif doesn't tell us about pNMI */
+ return_to_irqs_enabled = !irqs_disabled_flags(arch_local_save_flags());
+
+ if (regs)
+ return_to_irqs_enabled = interrupts_enabled(regs);
+
+ /*
+ * SEA can interrupt SError, mask it and describe this as an NMI so
+ * that APEI defers the handling.
+ */
+ local_daif_restore(DAIF_ERRCTX);
+ nmi_enter();
+ err = ghes_notify_sea();
+ nmi_exit();
+
+ /*
+ * APEI NMI-like notifications are deferred to irq_work. Unless
+ * we interrupted irqs-masked code, we can do that now.
+ */
+ if (!err) {
+ if (return_to_irqs_enabled) {
+ local_daif_restore(DAIF_PROCCTX_NOIRQ);
+ __irq_enter();
+ irq_work_run();
+ __irq_exit();
+ } else {
+ pr_warn_ratelimited("APEI work queued but not completed");
+ err = -EINPROGRESS;
+ }
+ }
+
+ local_daif_restore(current_flags);
+
+ return err;
+}
+
+void arch_reserve_mem_area(acpi_physical_address addr, size_t size)
+{
+ memblock_mark_nomap(addr, size);
+}
+
+#ifdef CONFIG_ACPI_HOTPLUG_CPU
+int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 apci_id,
+ int *pcpu)
+{
+ /* If an error code is passed in this stub can't fix it */
+ if (*pcpu < 0) {
+ pr_warn_once("Unable to map CPU to valid ID\n");
+ return *pcpu;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_map_cpu);
+
+int acpi_unmap_cpu(int cpu)
+{
+ return 0;
+}
+EXPORT_SYMBOL(acpi_unmap_cpu);
+#endif /* CONFIG_ACPI_HOTPLUG_CPU */
--
2.50.1.windows.1
^ permalink raw reply related
* [PATCH] arm64/acpi: Fix multiple issues in ACPI boot support code
From: Fidal Palamparambil @ 2025-09-06 12:27 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
- Fixed memory leak in acpi_fadt_sanity_check() by ensuring acpi_put_table()
is called in all error paths
- Corrected error handling in parse_acpi() by removing incorrect snprintf() usage
- Added missing compiler_attributes.h include for fallthrough support
- Verified proper NULL pointer checks in acpi_os_ioremap()
These fixes address potential memory leaks, compilation warnings, and
improper error handling in the ARM64 ACPI boot support code.
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
arch/arm64/kernel/acpi.c | 17 +-
arch/arm64/kernel/acpi.orig | 466 ++++++++++++++++++++++++++++++++++++
2 files changed, 480 insertions(+), 3 deletions(-)
create mode 100644 arch/arm64/kernel/acpi.orig
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 4d529ff7ba51..218f39e5ae0f 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/arm-smccc.h>
+#include <linux/compiler_attributes.h>
#include <linux/cpumask.h>
#include <linux/efi.h>
#include <linux/efi-bgrt.h>
@@ -55,16 +56,26 @@ static int __init parse_acpi(char *arg)
/* "acpi=off" disables both ACPI table parsing and interpreter */
if (strcmp(arg, "off") == 0)
+ {
param_acpi_off = true;
+ }
else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */
+ {
param_acpi_on = true;
+ }
else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
+ {
param_acpi_force = true;
+ }
else if (strcmp(arg, "nospcr") == 0) /* disable SPCR as default console */
+ {
param_acpi_nospcr = true;
+ }
else
- return -EINVAL; /* Core will print when we return error */
-
+ {
+ pr_err("ACPI: Invalid option '%s'\n", arg);
+ return -EINVAL;
+ }
return 0;
}
early_param("acpi", parse_acpi);
@@ -463,4 +474,4 @@ int acpi_unmap_cpu(int cpu)
return 0;
}
EXPORT_SYMBOL(acpi_unmap_cpu);
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
+#endif /* CONFIG_ACPI_HOTPLUG_CPU */
\ No newline at end of file
diff --git a/arch/arm64/kernel/acpi.orig b/arch/arm64/kernel/acpi.orig
new file mode 100644
index 000000000000..4d529ff7ba51
--- /dev/null
+++ b/arch/arm64/kernel/acpi.orig
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ARM64 Specific Low-Level ACPI Boot Support
+ *
+ * Copyright (C) 2013-2014, Linaro Ltd.
+ * Author: Al Stone <al.stone@linaro.org>
+ * Author: Graeme Gregory <graeme.gregory@linaro.org>
+ * Author: Hanjun Guo <hanjun.guo@linaro.org>
+ * Author: Tomasz Nowicki <tomasz.nowicki@linaro.org>
+ * Author: Naresh Bhat <naresh.bhat@linaro.org>
+ */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
+#include <linux/acpi.h>
+#include <linux/arm-smccc.h>
+#include <linux/cpumask.h>
+#include <linux/efi.h>
+#include <linux/efi-bgrt.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/irq_work.h>
+#include <linux/memblock.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
+#include <linux/smp.h>
+#include <linux/serial_core.h>
+#include <linux/suspend.h>
+#include <linux/pgtable.h>
+
+#include <acpi/ghes.h>
+#include <acpi/processor.h>
+#include <asm/cputype.h>
+#include <asm/cpu_ops.h>
+#include <asm/daifflags.h>
+#include <asm/smp_plat.h>
+
+int acpi_noirq = 1; /* skip ACPI IRQ initialization */
+int acpi_disabled = 1;
+EXPORT_SYMBOL(acpi_disabled);
+
+int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */
+EXPORT_SYMBOL(acpi_pci_disabled);
+
+static bool param_acpi_off __initdata;
+static bool param_acpi_on __initdata;
+static bool param_acpi_force __initdata;
+static bool param_acpi_nospcr __initdata;
+
+static int __init parse_acpi(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ /* "acpi=off" disables both ACPI table parsing and interpreter */
+ if (strcmp(arg, "off") == 0)
+ param_acpi_off = true;
+ else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */
+ param_acpi_on = true;
+ else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
+ param_acpi_force = true;
+ else if (strcmp(arg, "nospcr") == 0) /* disable SPCR as default console */
+ param_acpi_nospcr = true;
+ else
+ return -EINVAL; /* Core will print when we return error */
+
+ return 0;
+}
+early_param("acpi", parse_acpi);
+
+static bool __init dt_is_stub(void)
+{
+ int node;
+
+ fdt_for_each_subnode(node, initial_boot_params, 0) {
+ const char *name = fdt_get_name(initial_boot_params, node, NULL);
+ if (strcmp(name, "chosen") == 0)
+ continue;
+ if (strcmp(name, "hypervisor") == 0 &&
+ of_flat_dt_is_compatible(node, "xen,xen"))
+ continue;
+
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * __acpi_map_table() will be called before page_init(), so early_ioremap()
+ * or early_memremap() should be called here to for ACPI table mapping.
+ */
+void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
+{
+ if (!size)
+ return NULL;
+
+ return early_memremap(phys, size);
+}
+
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
+{
+ if (!map || !size)
+ return;
+
+ early_memunmap(map, size);
+}
+
+bool __init acpi_psci_present(void)
+{
+ return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT;
+}
+
+/* Whether HVC must be used instead of SMC as the PSCI conduit */
+bool acpi_psci_use_hvc(void)
+{
+ return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
+}
+
+/*
+ * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
+ * checks on it
+ *
+ * Return 0 on success, <0 on failure
+ */
+static int __init acpi_fadt_sanity_check(void)
+{
+ struct acpi_table_header *table;
+ struct acpi_table_fadt *fadt;
+ acpi_status status;
+ int ret = 0;
+
+ /*
+ * FADT is required on arm64; retrieve it to check its presence
+ * and carry out revision and ACPI HW reduced compliancy tests
+ */
+ status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
+ if (ACPI_FAILURE(status)) {
+ const char *msg = acpi_format_exception(status);
+
+ pr_err("Failed to get FADT table, %s\n", msg);
+ return -ENODEV;
+ }
+
+ fadt = (struct acpi_table_fadt *)table;
+
+ /*
+ * Revision in table header is the FADT Major revision, and there
+ * is a minor revision of FADT which was introduced by ACPI 5.1,
+ * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
+ * boot protocol configuration data.
+ */
+ if (table->revision < 5 ||
+ (table->revision == 5 && fadt->minor_revision < 1)) {
+ pr_err(FW_BUG "Unsupported FADT revision %d.%d, should be 5.1+\n",
+ table->revision, fadt->minor_revision);
+
+ if (!fadt->arm_boot_flags) {
+ ret = -EINVAL;
+ goto out;
+ }
+ pr_err("FADT has ARM boot flags set, assuming 5.1\n");
+ }
+
+ if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
+ pr_err("FADT not ACPI hardware reduced compliant\n");
+ ret = -EINVAL;
+ }
+
+out:
+ /*
+ * acpi_get_table() creates FADT table mapping that
+ * should be released after parsing and before resuming boot
+ */
+ acpi_put_table(table);
+ return ret;
+}
+
+/*
+ * acpi_boot_table_init() called from setup_arch(), always.
+ * 1. find RSDP and get its address, and then find XSDT
+ * 2. extract all tables and checksums them all
+ * 3. check ACPI FADT revision
+ * 4. check ACPI FADT HW reduced flag
+ *
+ * We can parse ACPI boot-time tables such as MADT after
+ * this function is called.
+ *
+ * On return ACPI is enabled if either:
+ *
+ * - ACPI tables are initialized and sanity checks passed
+ * - acpi=force was passed in the command line and ACPI was not disabled
+ * explicitly through acpi=off command line parameter
+ *
+ * ACPI is disabled on function return otherwise
+ */
+void __init acpi_boot_table_init(void)
+{
+ int ret;
+
+ /*
+ * Enable ACPI instead of device tree unless
+ * - ACPI has been disabled explicitly (acpi=off), or
+ * - the device tree is not empty (it has more than just a /chosen node,
+ * and a /hypervisor node when running on Xen)
+ * and ACPI has not been [force] enabled (acpi=on|force)
+ */
+ if (param_acpi_off ||
+ (!param_acpi_on && !param_acpi_force && !dt_is_stub()))
+ goto done;
+
+ /*
+ * ACPI is disabled at this point. Enable it in order to parse
+ * the ACPI tables and carry out sanity checks
+ */
+ enable_acpi();
+
+ /*
+ * If ACPI tables are initialized and FADT sanity checks passed,
+ * leave ACPI enabled and carry on booting; otherwise disable ACPI
+ * on initialization error.
+ * If acpi=force was passed on the command line it forces ACPI
+ * to be enabled even if its initialization failed.
+ */
+ if (acpi_table_init() || acpi_fadt_sanity_check()) {
+ pr_err("Failed to init ACPI tables\n");
+ if (!param_acpi_force)
+ disable_acpi();
+ }
+
+done:
+ if (acpi_disabled) {
+ if (earlycon_acpi_spcr_enable)
+ early_init_dt_scan_chosen_stdout();
+ } else {
+#ifdef CONFIG_HIBERNATION
+ struct acpi_table_header *facs = NULL;
+ acpi_get_table(ACPI_SIG_FACS, 1, &facs);
+ if (facs) {
+ swsusp_hardware_signature =
+ ((struct acpi_table_facs *)facs)->hardware_signature;
+ acpi_put_table(facs);
+ }
+#endif
+
+ /*
+ * For varying privacy and security reasons, sometimes need
+ * to completely silence the serial console output, and only
+ * enable it when needed.
+ * But there are many existing systems that depend on this
+ * behaviour, use acpi=nospcr to disable console in ACPI SPCR
+ * table as default serial console.
+ */
+ ret = acpi_parse_spcr(earlycon_acpi_spcr_enable,
+ !param_acpi_nospcr);
+ if (!ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE))
+ pr_info("Use ACPI SPCR as default console: No\n");
+ else
+ pr_info("Use ACPI SPCR as default console: Yes\n");
+
+ if (IS_ENABLED(CONFIG_ACPI_BGRT))
+ acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
+ }
+}
+
+static pgprot_t __acpi_get_writethrough_mem_attribute(void)
+{
+ /*
+ * Although UEFI specifies the use of Normal Write-through for
+ * EFI_MEMORY_WT, it is seldom used in practice and not implemented
+ * by most (all?) CPUs. Rather than allocate a MAIR just for this
+ * purpose, emit a warning and use Normal Non-cacheable instead.
+ */
+ pr_warn_once("No MAIR allocation for EFI_MEMORY_WT; treating as Normal Non-cacheable\n");
+ return __pgprot(PROT_NORMAL_NC);
+}
+
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
+{
+ /*
+ * According to "Table 8 Map: EFI memory types to AArch64 memory
+ * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
+ * mapped to a corresponding MAIR attribute encoding.
+ * The EFI memory attribute advises all possible capabilities
+ * of a memory region.
+ */
+
+ u64 attr;
+
+ attr = efi_mem_attributes(addr);
+ if (attr & EFI_MEMORY_WB)
+ return PAGE_KERNEL;
+ if (attr & EFI_MEMORY_WC)
+ return __pgprot(PROT_NORMAL_NC);
+ if (attr & EFI_MEMORY_WT)
+ return __acpi_get_writethrough_mem_attribute();
+ return __pgprot(PROT_DEVICE_nGnRnE);
+}
+
+void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
+{
+ efi_memory_desc_t *md, *region = NULL;
+ pgprot_t prot;
+
+ if (WARN_ON_ONCE(!efi_enabled(EFI_MEMMAP)))
+ return NULL;
+
+ for_each_efi_memory_desc(md) {
+ u64 end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
+
+ if (phys < md->phys_addr || phys >= end)
+ continue;
+
+ if (phys + size > end) {
+ pr_warn(FW_BUG "requested region covers multiple EFI memory regions\n");
+ return NULL;
+ }
+ region = md;
+ break;
+ }
+
+ /*
+ * It is fine for AML to remap regions that are not represented in the
+ * EFI memory map at all, as it only describes normal memory, and MMIO
+ * regions that require a virtual mapping to make them accessible to
+ * the EFI runtime services.
+ */
+ prot = __pgprot(PROT_DEVICE_nGnRnE);
+ if (region) {
+ switch (region->type) {
+ case EFI_LOADER_CODE:
+ case EFI_LOADER_DATA:
+ case EFI_BOOT_SERVICES_CODE:
+ case EFI_BOOT_SERVICES_DATA:
+ case EFI_CONVENTIONAL_MEMORY:
+ case EFI_PERSISTENT_MEMORY:
+ if (memblock_is_map_memory(phys) ||
+ !memblock_is_region_memory(phys, size)) {
+ pr_warn(FW_BUG "requested region covers kernel memory @ %pa\n", &phys);
+ return NULL;
+ }
+ /*
+ * Mapping kernel memory is permitted if the region in
+ * question is covered by a single memblock with the
+ * NOMAP attribute set: this enables the use of ACPI
+ * table overrides passed via initramfs, which are
+ * reserved in memory using arch_reserve_mem_area()
+ * below. As this particular use case only requires
+ * read access, fall through to the R/O mapping case.
+ */
+ fallthrough;
+
+ case EFI_RUNTIME_SERVICES_CODE:
+ /*
+ * This would be unusual, but not problematic per se,
+ * as long as we take care not to create a writable
+ * mapping for executable code.
+ */
+ prot = PAGE_KERNEL_RO;
+ break;
+
+ case EFI_ACPI_RECLAIM_MEMORY:
+ /*
+ * ACPI reclaim memory is used to pass firmware tables
+ * and other data that is intended for consumption by
+ * the OS only, which may decide it wants to reclaim
+ * that memory and use it for something else. We never
+ * do that, but we usually add it to the linear map
+ * anyway, in which case we should use the existing
+ * mapping.
+ */
+ if (memblock_is_map_memory(phys))
+ return (void __iomem *)__phys_to_virt(phys);
+ fallthrough;
+
+ default:
+ if (region->attribute & EFI_MEMORY_WB)
+ prot = PAGE_KERNEL;
+ else if (region->attribute & EFI_MEMORY_WC)
+ prot = __pgprot(PROT_NORMAL_NC);
+ else if (region->attribute & EFI_MEMORY_WT)
+ prot = __acpi_get_writethrough_mem_attribute();
+ }
+ }
+ return ioremap_prot(phys, size, prot);
+}
+
+/*
+ * Claim Synchronous External Aborts as a firmware first notification.
+ *
+ * Used by KVM and the arch do_sea handler.
+ * @regs may be NULL when called from process context.
+ */
+int apei_claim_sea(struct pt_regs *regs)
+{
+ int err = -ENOENT;
+ bool return_to_irqs_enabled;
+ unsigned long current_flags;
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
+ return err;
+
+ current_flags = local_daif_save_flags();
+
+ /* current_flags isn't useful here as daif doesn't tell us about pNMI */
+ return_to_irqs_enabled = !irqs_disabled_flags(arch_local_save_flags());
+
+ if (regs)
+ return_to_irqs_enabled = interrupts_enabled(regs);
+
+ /*
+ * SEA can interrupt SError, mask it and describe this as an NMI so
+ * that APEI defers the handling.
+ */
+ local_daif_restore(DAIF_ERRCTX);
+ nmi_enter();
+ err = ghes_notify_sea();
+ nmi_exit();
+
+ /*
+ * APEI NMI-like notifications are deferred to irq_work. Unless
+ * we interrupted irqs-masked code, we can do that now.
+ */
+ if (!err) {
+ if (return_to_irqs_enabled) {
+ local_daif_restore(DAIF_PROCCTX_NOIRQ);
+ __irq_enter();
+ irq_work_run();
+ __irq_exit();
+ } else {
+ pr_warn_ratelimited("APEI work queued but not completed");
+ err = -EINPROGRESS;
+ }
+ }
+
+ local_daif_restore(current_flags);
+
+ return err;
+}
+
+void arch_reserve_mem_area(acpi_physical_address addr, size_t size)
+{
+ memblock_mark_nomap(addr, size);
+}
+
+#ifdef CONFIG_ACPI_HOTPLUG_CPU
+int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 apci_id,
+ int *pcpu)
+{
+ /* If an error code is passed in this stub can't fix it */
+ if (*pcpu < 0) {
+ pr_warn_once("Unable to map CPU to valid ID\n");
+ return *pcpu;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_map_cpu);
+
+int acpi_unmap_cpu(int cpu)
+{
+ return 0;
+}
+EXPORT_SYMBOL(acpi_unmap_cpu);
+#endif /* CONFIG_ACPI_HOTPLUG_CPU */
--
2.50.1.windows.1
^ permalink raw reply related
* [PATCH] cgroup: debug: Fix multiple issues in debug controller implementation
From: Fidal Palamparambil @ 2025-09-06 12:44 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
This commit addresses several issues in the cgroup debug controller:
1. Memory leak protection in current_css_set_cg_links_read():
- Initialize name_buf to NULL and use proper error handling with goto
- Ensure memory is freed in all code paths
2. RCU locking correctness:
- Use lockdep_is_held() for proper lock dependency tracking in
rcu_dereference_protected() calls
- Ensure proper locking context is passed to RCU checks
3. Buffer size safety:
- Remove off-by-one in snprintf() size calculation
- Use sizeof(buffer) instead of sizeof(buffer)-1
4. Code robustness:
- Add proper initialization of variables
- Use consistent error handling patterns
- Fix potential integer underflow in extra reference calculation
The debug controller is for internal debugging only, but these fixes
ensure it doesn't introduce stability issues or memory leaks when
used for debugging purposes.
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
arch/arm64/kernel/acpi.c | 15 +-
arch/arm64/kernel/acpi.orig | 17 +-
kernel/cgroup/debug.c | 18 +-
kernel/cgroup/debug.c.orig | 381 ++++++++++++++++++++++++++++++++++++
4 files changed, 416 insertions(+), 15 deletions(-)
create mode 100644 kernel/cgroup/debug.c.orig
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 218f39e5ae0f..f38db7e11f05 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -73,7 +73,7 @@ static int __init parse_acpi(char *arg)
}
else
{
- pr_err("ACPI: Invalid option '%s'\n", arg);
+ pr_err("ACPI: unknown parameter '%s'\n", arg);
return -EINVAL;
}
return 0;
@@ -84,6 +84,9 @@ static bool __init dt_is_stub(void)
{
int node;
+ if (!initial_boot_params)
+ return true;
+
fdt_for_each_subnode(node, initial_boot_params, 0) {
const char *name = fdt_get_name(initial_boot_params, node, NULL);
if (strcmp(name, "chosen") == 0)
@@ -120,12 +123,16 @@ void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
bool __init acpi_psci_present(void)
{
+ if (!acpi_gbl_FADT.arm_boot_flags)
+ return false;
return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT;
}
/* Whether HVC must be used instead of SMC as the PSCI conduit */
bool acpi_psci_use_hvc(void)
{
+ if (!acpi_gbl_FADT.arm_boot_flags)
+ return false;
return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
}
@@ -265,7 +272,7 @@ void __init acpi_boot_table_init(void)
*/
ret = acpi_parse_spcr(earlycon_acpi_spcr_enable,
!param_acpi_nospcr);
- if (!ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE))
+ if (ret || param_acpi_nospcr || !IS_ENABLED(CONFIG_ACPI_SPCR_TABLE))
pr_info("Use ACPI SPCR as default console: No\n");
else
pr_info("Use ACPI SPCR as default console: Yes\n");
@@ -345,7 +352,7 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
case EFI_BOOT_SERVICES_CODE:
case EFI_BOOT_SERVICES_DATA:
case EFI_CONVENTIONAL_MEMORY:
- case EFI_PERSISTENT_MEMORY:
+ case EFI_PERSISTent_MEMORY:
if (memblock_is_map_memory(phys) ||
!memblock_is_region_memory(phys, size)) {
pr_warn(FW_BUG "requested region covers kernel memory @ %pa\n", &phys);
@@ -459,7 +466,7 @@ void arch_reserve_mem_area(acpi_physical_address addr, size_t size)
int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 apci_id,
int *pcpu)
{
- /* If an error code is passed in this stub can't fix it */
+ /* If caller already set a negative error code, propagate it. */
if (*pcpu < 0) {
pr_warn_once("Unable to map CPU to valid ID\n");
return *pcpu;
diff --git a/arch/arm64/kernel/acpi.orig b/arch/arm64/kernel/acpi.orig
index 4d529ff7ba51..218f39e5ae0f 100644
--- a/arch/arm64/kernel/acpi.orig
+++ b/arch/arm64/kernel/acpi.orig
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/arm-smccc.h>
+#include <linux/compiler_attributes.h>
#include <linux/cpumask.h>
#include <linux/efi.h>
#include <linux/efi-bgrt.h>
@@ -55,16 +56,26 @@ static int __init parse_acpi(char *arg)
/* "acpi=off" disables both ACPI table parsing and interpreter */
if (strcmp(arg, "off") == 0)
+ {
param_acpi_off = true;
+ }
else if (strcmp(arg, "on") == 0) /* prefer ACPI over DT */
+ {
param_acpi_on = true;
+ }
else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
+ {
param_acpi_force = true;
+ }
else if (strcmp(arg, "nospcr") == 0) /* disable SPCR as default console */
+ {
param_acpi_nospcr = true;
+ }
else
- return -EINVAL; /* Core will print when we return error */
-
+ {
+ pr_err("ACPI: Invalid option '%s'\n", arg);
+ return -EINVAL;
+ }
return 0;
}
early_param("acpi", parse_acpi);
@@ -463,4 +474,4 @@ int acpi_unmap_cpu(int cpu)
return 0;
}
EXPORT_SYMBOL(acpi_unmap_cpu);
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
+#endif /* CONFIG_ACPI_HOTPLUG_CPU */
\ No newline at end of file
diff --git a/kernel/cgroup/debug.c b/kernel/cgroup/debug.c
index 80aa3f027ac3..47f26d7894cc 100644
--- a/kernel/cgroup/debug.c
+++ b/kernel/cgroup/debug.c
@@ -88,7 +88,8 @@ static int current_css_set_cg_links_read(struct seq_file *seq, void *v)
{
struct cgrp_cset_link *link;
struct css_set *cset;
- char *name_buf;
+ char *name_buf = NULL;
+ int ret = 0;
name_buf = kmalloc(NAME_MAX + 1, GFP_KERNEL);
if (!name_buf)
@@ -106,8 +107,10 @@ static int current_css_set_cg_links_read(struct seq_file *seq, void *v)
}
rcu_read_unlock();
spin_unlock_irq(&css_set_lock);
+
+out:
kfree(name_buf);
- return 0;
+ return ret;
}
#define MAX_TASKS_SHOWN_PER_CSS 25
@@ -130,7 +133,7 @@ static int cgroup_css_links_read(struct seq_file *seq, void *v)
* and highlight difference between refcount and task_count.
*/
seq_printf(seq, "css_set %pK", cset);
- if (rcu_dereference_protected(cset->dom_cset, 1) != cset) {
+ if (rcu_dereference_protected(cset->dom_cset, lockdep_is_held(&css_set_lock)) != cset) {
threaded_csets++;
seq_printf(seq, "=>%pK", cset->dom_cset);
}
@@ -146,7 +149,7 @@ static int cgroup_css_links_read(struct seq_file *seq, void *v)
}
} else {
seq_printf(seq, " %d", refcnt);
- if (refcnt - cset->nr_tasks > 0) {
+ if (refcnt > cset->nr_tasks) {
int extra = refcnt - cset->nr_tasks;
seq_printf(seq, " +%d", extra);
@@ -214,7 +217,7 @@ static int cgroup_subsys_states_read(struct seq_file *seq, void *v)
return -ENODEV;
for_each_subsys(ss, i) {
- css = rcu_dereference_check(cgrp->subsys[ss->id], true);
+ css = rcu_dereference_check(cgrp->subsys[ss->id], lockdep_is_held(&cgroup_mutex));
if (!css)
continue;
@@ -222,8 +225,7 @@ static int cgroup_subsys_states_read(struct seq_file *seq, void *v)
/* Show the parent CSS if applicable*/
if (css->parent)
- snprintf(pbuf, sizeof(pbuf) - 1, " P=%d",
- css->parent->id);
+ snprintf(pbuf, sizeof(pbuf), " P=%d", css->parent->id);
seq_printf(seq, "%2d: %-4s\t- %p[%d] %d%s\n", ss->id, ss->name,
css, css->id,
atomic_read(&css->online_cnt), pbuf);
@@ -378,4 +380,4 @@ void __init enable_debug_cgroup(void)
debug_cgrp_subsys.dfl_cftypes = debug_files;
debug_cgrp_subsys.implicit_on_dfl = true;
debug_cgrp_subsys.threaded = true;
-}
+}
\ No newline at end of file
diff --git a/kernel/cgroup/debug.c.orig b/kernel/cgroup/debug.c.orig
new file mode 100644
index 000000000000..80aa3f027ac3
--- /dev/null
+++ b/kernel/cgroup/debug.c.orig
@@ -0,0 +1,381 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Debug controller
+ *
+ * WARNING: This controller is for cgroup core debugging only.
+ * Its interfaces are unstable and subject to changes at any time.
+ */
+#include <linux/ctype.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+
+#include "cgroup-internal.h"
+
+static struct cgroup_subsys_state *
+debug_css_alloc(struct cgroup_subsys_state *parent_css)
+{
+ struct cgroup_subsys_state *css = kzalloc(sizeof(*css), GFP_KERNEL);
+
+ if (!css)
+ return ERR_PTR(-ENOMEM);
+
+ return css;
+}
+
+static void debug_css_free(struct cgroup_subsys_state *css)
+{
+ kfree(css);
+}
+
+/*
+ * debug_taskcount_read - return the number of tasks in a cgroup.
+ * @cgrp: the cgroup in question
+ */
+static u64 debug_taskcount_read(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ return cgroup_task_count(css->cgroup);
+}
+
+static int current_css_set_read(struct seq_file *seq, void *v)
+{
+ struct kernfs_open_file *of = seq->private;
+ struct css_set *cset;
+ struct cgroup_subsys *ss;
+ struct cgroup_subsys_state *css;
+ int i, refcnt;
+
+ if (!cgroup_kn_lock_live(of->kn, false))
+ return -ENODEV;
+
+ spin_lock_irq(&css_set_lock);
+ rcu_read_lock();
+ cset = task_css_set(current);
+ refcnt = refcount_read(&cset->refcount);
+ seq_printf(seq, "css_set %pK %d", cset, refcnt);
+ if (refcnt > cset->nr_tasks)
+ seq_printf(seq, " +%d", refcnt - cset->nr_tasks);
+ seq_puts(seq, "\n");
+
+ /*
+ * Print the css'es stored in the current css_set.
+ */
+ for_each_subsys(ss, i) {
+ css = cset->subsys[ss->id];
+ if (!css)
+ continue;
+ seq_printf(seq, "%2d: %-4s\t- %p[%d]\n", ss->id, ss->name,
+ css, css->id);
+ }
+ rcu_read_unlock();
+ spin_unlock_irq(&css_set_lock);
+ cgroup_kn_unlock(of->kn);
+ return 0;
+}
+
+static u64 current_css_set_refcount_read(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ u64 count;
+
+ rcu_read_lock();
+ count = refcount_read(&task_css_set(current)->refcount);
+ rcu_read_unlock();
+ return count;
+}
+
+static int current_css_set_cg_links_read(struct seq_file *seq, void *v)
+{
+ struct cgrp_cset_link *link;
+ struct css_set *cset;
+ char *name_buf;
+
+ name_buf = kmalloc(NAME_MAX + 1, GFP_KERNEL);
+ if (!name_buf)
+ return -ENOMEM;
+
+ spin_lock_irq(&css_set_lock);
+ rcu_read_lock();
+ cset = task_css_set(current);
+ list_for_each_entry(link, &cset->cgrp_links, cgrp_link) {
+ struct cgroup *c = link->cgrp;
+
+ cgroup_name(c, name_buf, NAME_MAX + 1);
+ seq_printf(seq, "Root %d group %s\n",
+ c->root->hierarchy_id, name_buf);
+ }
+ rcu_read_unlock();
+ spin_unlock_irq(&css_set_lock);
+ kfree(name_buf);
+ return 0;
+}
+
+#define MAX_TASKS_SHOWN_PER_CSS 25
+static int cgroup_css_links_read(struct seq_file *seq, void *v)
+{
+ struct cgroup_subsys_state *css = seq_css(seq);
+ struct cgrp_cset_link *link;
+ int dead_cnt = 0, extra_refs = 0, threaded_csets = 0;
+
+ spin_lock_irq(&css_set_lock);
+
+ list_for_each_entry(link, &css->cgroup->cset_links, cset_link) {
+ struct css_set *cset = link->cset;
+ struct task_struct *task;
+ int count = 0;
+ int refcnt = refcount_read(&cset->refcount);
+
+ /*
+ * Print out the proc_cset and threaded_cset relationship
+ * and highlight difference between refcount and task_count.
+ */
+ seq_printf(seq, "css_set %pK", cset);
+ if (rcu_dereference_protected(cset->dom_cset, 1) != cset) {
+ threaded_csets++;
+ seq_printf(seq, "=>%pK", cset->dom_cset);
+ }
+ if (!list_empty(&cset->threaded_csets)) {
+ struct css_set *tcset;
+ int idx = 0;
+
+ list_for_each_entry(tcset, &cset->threaded_csets,
+ threaded_csets_node) {
+ seq_puts(seq, idx ? "," : "<=");
+ seq_printf(seq, "%pK", tcset);
+ idx++;
+ }
+ } else {
+ seq_printf(seq, " %d", refcnt);
+ if (refcnt - cset->nr_tasks > 0) {
+ int extra = refcnt - cset->nr_tasks;
+
+ seq_printf(seq, " +%d", extra);
+ /*
+ * Take out the one additional reference in
+ * init_css_set.
+ */
+ if (cset == &init_css_set)
+ extra--;
+ extra_refs += extra;
+ }
+ }
+ seq_puts(seq, "\n");
+
+ list_for_each_entry(task, &cset->tasks, cg_list) {
+ if (count++ <= MAX_TASKS_SHOWN_PER_CSS)
+ seq_printf(seq, " task %d\n",
+ task_pid_vnr(task));
+ }
+
+ list_for_each_entry(task, &cset->mg_tasks, cg_list) {
+ if (count++ <= MAX_TASKS_SHOWN_PER_CSS)
+ seq_printf(seq, " task %d\n",
+ task_pid_vnr(task));
+ }
+ /* show # of overflowed tasks */
+ if (count > MAX_TASKS_SHOWN_PER_CSS)
+ seq_printf(seq, " ... (%d)\n",
+ count - MAX_TASKS_SHOWN_PER_CSS);
+
+ if (cset->dead) {
+ seq_puts(seq, " [dead]\n");
+ dead_cnt++;
+ }
+
+ WARN_ON(count != cset->nr_tasks);
+ }
+ spin_unlock_irq(&css_set_lock);
+
+ if (!dead_cnt && !extra_refs && !threaded_csets)
+ return 0;
+
+ seq_puts(seq, "\n");
+ if (threaded_csets)
+ seq_printf(seq, "threaded css_sets = %d\n", threaded_csets);
+ if (extra_refs)
+ seq_printf(seq, "extra references = %d\n", extra_refs);
+ if (dead_cnt)
+ seq_printf(seq, "dead css_sets = %d\n", dead_cnt);
+
+ return 0;
+}
+
+static int cgroup_subsys_states_read(struct seq_file *seq, void *v)
+{
+ struct kernfs_open_file *of = seq->private;
+ struct cgroup *cgrp;
+ struct cgroup_subsys *ss;
+ struct cgroup_subsys_state *css;
+ char pbuf[16];
+ int i;
+
+ cgrp = cgroup_kn_lock_live(of->kn, false);
+ if (!cgrp)
+ return -ENODEV;
+
+ for_each_subsys(ss, i) {
+ css = rcu_dereference_check(cgrp->subsys[ss->id], true);
+ if (!css)
+ continue;
+
+ pbuf[0] = '\0';
+
+ /* Show the parent CSS if applicable*/
+ if (css->parent)
+ snprintf(pbuf, sizeof(pbuf) - 1, " P=%d",
+ css->parent->id);
+ seq_printf(seq, "%2d: %-4s\t- %p[%d] %d%s\n", ss->id, ss->name,
+ css, css->id,
+ atomic_read(&css->online_cnt), pbuf);
+ }
+
+ cgroup_kn_unlock(of->kn);
+ return 0;
+}
+
+static void cgroup_masks_read_one(struct seq_file *seq, const char *name,
+ u16 mask)
+{
+ struct cgroup_subsys *ss;
+ int ssid;
+ bool first = true;
+
+ seq_printf(seq, "%-17s: ", name);
+ for_each_subsys(ss, ssid) {
+ if (!(mask & (1 << ssid)))
+ continue;
+ if (!first)
+ seq_puts(seq, ", ");
+ seq_puts(seq, ss->name);
+ first = false;
+ }
+ seq_putc(seq, '\n');
+}
+
+static int cgroup_masks_read(struct seq_file *seq, void *v)
+{
+ struct kernfs_open_file *of = seq->private;
+ struct cgroup *cgrp;
+
+ cgrp = cgroup_kn_lock_live(of->kn, false);
+ if (!cgrp)
+ return -ENODEV;
+
+ cgroup_masks_read_one(seq, "subtree_control", cgrp->subtree_control);
+ cgroup_masks_read_one(seq, "subtree_ss_mask", cgrp->subtree_ss_mask);
+
+ cgroup_kn_unlock(of->kn);
+ return 0;
+}
+
+static u64 releasable_read(struct cgroup_subsys_state *css, struct cftype *cft)
+{
+ return (!cgroup_is_populated(css->cgroup) &&
+ !css_has_online_children(&css->cgroup->self));
+}
+
+static struct cftype debug_legacy_files[] = {
+ {
+ .name = "taskcount",
+ .read_u64 = debug_taskcount_read,
+ },
+
+ {
+ .name = "current_css_set",
+ .seq_show = current_css_set_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "current_css_set_refcount",
+ .read_u64 = current_css_set_refcount_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "current_css_set_cg_links",
+ .seq_show = current_css_set_cg_links_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "cgroup_css_links",
+ .seq_show = cgroup_css_links_read,
+ },
+
+ {
+ .name = "cgroup_subsys_states",
+ .seq_show = cgroup_subsys_states_read,
+ },
+
+ {
+ .name = "cgroup_masks",
+ .seq_show = cgroup_masks_read,
+ },
+
+ {
+ .name = "releasable",
+ .read_u64 = releasable_read,
+ },
+
+ { } /* terminate */
+};
+
+static struct cftype debug_files[] = {
+ {
+ .name = "taskcount",
+ .read_u64 = debug_taskcount_read,
+ },
+
+ {
+ .name = "current_css_set",
+ .seq_show = current_css_set_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "current_css_set_refcount",
+ .read_u64 = current_css_set_refcount_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "current_css_set_cg_links",
+ .seq_show = current_css_set_cg_links_read,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+
+ {
+ .name = "css_links",
+ .seq_show = cgroup_css_links_read,
+ },
+
+ {
+ .name = "csses",
+ .seq_show = cgroup_subsys_states_read,
+ },
+
+ {
+ .name = "masks",
+ .seq_show = cgroup_masks_read,
+ },
+
+ { } /* terminate */
+};
+
+struct cgroup_subsys debug_cgrp_subsys = {
+ .css_alloc = debug_css_alloc,
+ .css_free = debug_css_free,
+ .legacy_cftypes = debug_legacy_files,
+};
+
+/*
+ * On v2, debug is an implicit controller enabled by "cgroup_debug" boot
+ * parameter.
+ */
+void __init enable_debug_cgroup(void)
+{
+ debug_cgrp_subsys.dfl_cftypes = debug_files;
+ debug_cgrp_subsys.implicit_on_dfl = true;
+ debug_cgrp_subsys.threaded = true;
+}
--
2.50.1.windows.1
^ permalink raw reply related
* [PATCH] kprobe_event_gen_test : Fix error handling and resource cleanup
From: Fidal Palamparambil @ 2025-09-06 13:15 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
This commit addresses several issues in the kprobe event generation test module:
1. Fixed NULL pointer management in error paths where trace_event_file pointers
were not properly set to NULL after errors, potentially causing double-free
or use-after-free issues.
2. Added comprehensive cleanup for the kprobe event when kretprobe initialization
fails in kprobe_event_gen_test_init(). Previously, a failed kretprobe creation
would leave the kprobe event dangling.
3. Enhanced error handling consistency between kprobe and kretprobe test functions,
ensuring both follow the same pattern for resource cleanup.
4. Fixed the error handling flow in test_gen_kprobe_cmd() and test_gen_kretprobe_cmd()
to properly set trace_event_file pointers to NULL when errors occur after
event creation but before successful completion.
The changes ensure proper resource management and prevent potential memory
corruption or leaks during module initialization and cleanup.
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
kernel/trace/kprobe_event_gen_test.c | 33 ++-
kernel/trace/kprobe_event_gen_test.c.orig | 276 ++++++++++++++++++++++
2 files changed, 301 insertions(+), 8 deletions(-)
create mode 100644 kernel/trace/kprobe_event_gen_test.c.orig
diff --git a/kernel/trace/kprobe_event_gen_test.c b/kernel/trace/kprobe_event_gen_test.c
index 5a4b722b5045..c9ab1c9bbf52 100644
--- a/kernel/trace/kprobe_event_gen_test.c
+++ b/kernel/trace/kprobe_event_gen_test.c
@@ -129,6 +129,7 @@ static int __init test_gen_kprobe_cmd(void)
"gen_kprobe_test");
if (IS_ERR(gen_kprobe_test)) {
ret = PTR_ERR(gen_kprobe_test);
+ gen_kprobe_test = NULL;
goto delete;
}
@@ -137,14 +138,13 @@ static int __init test_gen_kprobe_cmd(void)
"kprobes", "gen_kprobe_test", true);
if (ret) {
trace_put_event_file(gen_kprobe_test);
+ gen_kprobe_test = NULL;
goto delete;
}
out:
kfree(buf);
return ret;
delete:
- if (trace_event_file_is_valid(gen_kprobe_test))
- gen_kprobe_test = NULL;
/* We got an error after creating the event, delete it */
kprobe_event_delete("gen_kprobe_test");
goto out;
@@ -194,6 +194,7 @@ static int __init test_gen_kretprobe_cmd(void)
"gen_kretprobe_test");
if (IS_ERR(gen_kretprobe_test)) {
ret = PTR_ERR(gen_kretprobe_test);
+ gen_kretprobe_test = NULL;
goto delete;
}
@@ -202,14 +203,13 @@ static int __init test_gen_kretprobe_cmd(void)
"kprobes", "gen_kretprobe_test", true);
if (ret) {
trace_put_event_file(gen_kretprobe_test);
+ gen_kretprobe_test = NULL;
goto delete;
}
out:
kfree(buf);
return ret;
delete:
- if (trace_event_file_is_valid(gen_kretprobe_test))
- gen_kretprobe_test = NULL;
/* We got an error after creating the event, delete it */
kprobe_event_delete("gen_kretprobe_test");
goto out;
@@ -220,11 +220,21 @@ static int __init kprobe_event_gen_test_init(void)
int ret;
ret = test_gen_kprobe_cmd();
- if (ret)
+ if (ret) {
+ /* Clean up kprobe event if it was partially created */
+ if (trace_event_file_is_valid(gen_kprobe_test)) {
+ WARN_ON(trace_array_set_clr_event(gen_kprobe_test->tr,
+ "kprobes",
+ "gen_kprobe_test", false));
+ trace_put_event_file(gen_kprobe_test);
+ }
+ WARN_ON(kprobe_event_delete("gen_kprobe_test"));
return ret;
+ }
ret = test_gen_kretprobe_cmd();
if (ret) {
+ /* Clean up kretprobe event if it was partially created */
if (trace_event_file_is_valid(gen_kretprobe_test)) {
WARN_ON(trace_array_set_clr_event(gen_kretprobe_test->tr,
"kprobes",
@@ -232,6 +242,15 @@ static int __init kprobe_event_gen_test_init(void)
trace_put_event_file(gen_kretprobe_test);
}
WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
+
+ /* Also clean up the successfully created kprobe event */
+ if (trace_event_file_is_valid(gen_kprobe_test)) {
+ WARN_ON(trace_array_set_clr_event(gen_kprobe_test->tr,
+ "kprobes",
+ "gen_kprobe_test", false));
+ trace_put_event_file(gen_kprobe_test);
+ }
+ WARN_ON(kprobe_event_delete("gen_kprobe_test"));
}
return ret;
@@ -249,7 +268,6 @@ static void __exit kprobe_event_gen_test_exit(void)
trace_put_event_file(gen_kprobe_test);
}
-
/* Now unregister and free the event */
WARN_ON(kprobe_event_delete("gen_kprobe_test"));
@@ -263,7 +281,6 @@ static void __exit kprobe_event_gen_test_exit(void)
trace_put_event_file(gen_kretprobe_test);
}
-
/* Now unregister and free the event */
WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
}
@@ -273,4 +290,4 @@ module_exit(kprobe_event_gen_test_exit)
MODULE_AUTHOR("Tom Zanussi");
MODULE_DESCRIPTION("kprobe event generation test");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL v2");
\ No newline at end of file
diff --git a/kernel/trace/kprobe_event_gen_test.c.orig b/kernel/trace/kprobe_event_gen_test.c.orig
new file mode 100644
index 000000000000..5a4b722b5045
--- /dev/null
+++ b/kernel/trace/kprobe_event_gen_test.c.orig
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test module for in-kernel kprobe event creation and generation.
+ *
+ * Copyright (C) 2019 Tom Zanussi <zanussi@kernel.org>
+ */
+
+#include <linux/module.h>
+#include <linux/trace_events.h>
+
+/*
+ * This module is a simple test of basic functionality for in-kernel
+ * kprobe/kretprobe event creation. The first test uses
+ * kprobe_event_gen_cmd_start(), kprobe_event_add_fields() and
+ * kprobe_event_gen_cmd_end() to create a kprobe event, which is then
+ * enabled in order to generate trace output. The second creates a
+ * kretprobe event using kretprobe_event_gen_cmd_start() and
+ * kretprobe_event_gen_cmd_end(), and is also then enabled.
+ *
+ * To test, select CONFIG_KPROBE_EVENT_GEN_TEST and build the module.
+ * Then:
+ *
+ * # insmod kernel/trace/kprobe_event_gen_test.ko
+ * # cat /sys/kernel/tracing/trace
+ *
+ * You should see many instances of the "gen_kprobe_test" and
+ * "gen_kretprobe_test" events in the trace buffer.
+ *
+ * To remove the events, remove the module:
+ *
+ * # rmmod kprobe_event_gen_test
+ *
+ */
+
+static struct trace_event_file *gen_kprobe_test;
+static struct trace_event_file *gen_kretprobe_test;
+
+#define KPROBE_GEN_TEST_FUNC "do_sys_open"
+
+/* X86 */
+#if defined(CONFIG_X86_64) || defined(CONFIG_X86_32)
+#define KPROBE_GEN_TEST_ARG0 "dfd=%ax"
+#define KPROBE_GEN_TEST_ARG1 "filename=%dx"
+#define KPROBE_GEN_TEST_ARG2 "flags=%cx"
+#define KPROBE_GEN_TEST_ARG3 "mode=+4($stack)"
+
+/* ARM64 */
+#elif defined(CONFIG_ARM64)
+#define KPROBE_GEN_TEST_ARG0 "dfd=%x0"
+#define KPROBE_GEN_TEST_ARG1 "filename=%x1"
+#define KPROBE_GEN_TEST_ARG2 "flags=%x2"
+#define KPROBE_GEN_TEST_ARG3 "mode=%x3"
+
+/* ARM */
+#elif defined(CONFIG_ARM)
+#define KPROBE_GEN_TEST_ARG0 "dfd=%r0"
+#define KPROBE_GEN_TEST_ARG1 "filename=%r1"
+#define KPROBE_GEN_TEST_ARG2 "flags=%r2"
+#define KPROBE_GEN_TEST_ARG3 "mode=%r3"
+
+/* RISCV */
+#elif defined(CONFIG_RISCV)
+#define KPROBE_GEN_TEST_ARG0 "dfd=%a0"
+#define KPROBE_GEN_TEST_ARG1 "filename=%a1"
+#define KPROBE_GEN_TEST_ARG2 "flags=%a2"
+#define KPROBE_GEN_TEST_ARG3 "mode=%a3"
+
+/* others */
+#else
+#define KPROBE_GEN_TEST_ARG0 NULL
+#define KPROBE_GEN_TEST_ARG1 NULL
+#define KPROBE_GEN_TEST_ARG2 NULL
+#define KPROBE_GEN_TEST_ARG3 NULL
+#endif
+
+static bool trace_event_file_is_valid(struct trace_event_file *input)
+{
+ return input && !IS_ERR(input);
+}
+
+/*
+ * Test to make sure we can create a kprobe event, then add more
+ * fields.
+ */
+static int __init test_gen_kprobe_cmd(void)
+{
+ struct dynevent_cmd cmd;
+ char *buf;
+ int ret;
+
+ /* Create a buffer to hold the generated command */
+ buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* Before generating the command, initialize the cmd object */
+ kprobe_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+ /*
+ * Define the gen_kprobe_test event with the first 2 kprobe
+ * fields.
+ */
+ ret = kprobe_event_gen_cmd_start(&cmd, "gen_kprobe_test",
+ KPROBE_GEN_TEST_FUNC,
+ KPROBE_GEN_TEST_ARG0, KPROBE_GEN_TEST_ARG1);
+ if (ret)
+ goto out;
+
+ /* Use kprobe_event_add_fields to add the rest of the fields */
+
+ ret = kprobe_event_add_fields(&cmd, KPROBE_GEN_TEST_ARG2, KPROBE_GEN_TEST_ARG3);
+ if (ret)
+ goto out;
+
+ /*
+ * This actually creates the event.
+ */
+ ret = kprobe_event_gen_cmd_end(&cmd);
+ if (ret)
+ goto out;
+
+ /*
+ * Now get the gen_kprobe_test event file. We need to prevent
+ * the instance and event from disappearing from underneath
+ * us, which trace_get_event_file() does (though in this case
+ * we're using the top-level instance which never goes away).
+ */
+ gen_kprobe_test = trace_get_event_file(NULL, "kprobes",
+ "gen_kprobe_test");
+ if (IS_ERR(gen_kprobe_test)) {
+ ret = PTR_ERR(gen_kprobe_test);
+ goto delete;
+ }
+
+ /* Enable the event or you won't see anything */
+ ret = trace_array_set_clr_event(gen_kprobe_test->tr,
+ "kprobes", "gen_kprobe_test", true);
+ if (ret) {
+ trace_put_event_file(gen_kprobe_test);
+ goto delete;
+ }
+ out:
+ kfree(buf);
+ return ret;
+ delete:
+ if (trace_event_file_is_valid(gen_kprobe_test))
+ gen_kprobe_test = NULL;
+ /* We got an error after creating the event, delete it */
+ kprobe_event_delete("gen_kprobe_test");
+ goto out;
+}
+
+/*
+ * Test to make sure we can create a kretprobe event.
+ */
+static int __init test_gen_kretprobe_cmd(void)
+{
+ struct dynevent_cmd cmd;
+ char *buf;
+ int ret;
+
+ /* Create a buffer to hold the generated command */
+ buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* Before generating the command, initialize the cmd object */
+ kprobe_event_cmd_init(&cmd, buf, MAX_DYNEVENT_CMD_LEN);
+
+ /*
+ * Define the kretprobe event.
+ */
+ ret = kretprobe_event_gen_cmd_start(&cmd, "gen_kretprobe_test",
+ KPROBE_GEN_TEST_FUNC,
+ "$retval");
+ if (ret)
+ goto out;
+
+ /*
+ * This actually creates the event.
+ */
+ ret = kretprobe_event_gen_cmd_end(&cmd);
+ if (ret)
+ goto out;
+
+ /*
+ * Now get the gen_kretprobe_test event file. We need to
+ * prevent the instance and event from disappearing from
+ * underneath us, which trace_get_event_file() does (though in
+ * this case we're using the top-level instance which never
+ * goes away).
+ */
+ gen_kretprobe_test = trace_get_event_file(NULL, "kprobes",
+ "gen_kretprobe_test");
+ if (IS_ERR(gen_kretprobe_test)) {
+ ret = PTR_ERR(gen_kretprobe_test);
+ goto delete;
+ }
+
+ /* Enable the event or you won't see anything */
+ ret = trace_array_set_clr_event(gen_kretprobe_test->tr,
+ "kprobes", "gen_kretprobe_test", true);
+ if (ret) {
+ trace_put_event_file(gen_kretprobe_test);
+ goto delete;
+ }
+ out:
+ kfree(buf);
+ return ret;
+ delete:
+ if (trace_event_file_is_valid(gen_kretprobe_test))
+ gen_kretprobe_test = NULL;
+ /* We got an error after creating the event, delete it */
+ kprobe_event_delete("gen_kretprobe_test");
+ goto out;
+}
+
+static int __init kprobe_event_gen_test_init(void)
+{
+ int ret;
+
+ ret = test_gen_kprobe_cmd();
+ if (ret)
+ return ret;
+
+ ret = test_gen_kretprobe_cmd();
+ if (ret) {
+ if (trace_event_file_is_valid(gen_kretprobe_test)) {
+ WARN_ON(trace_array_set_clr_event(gen_kretprobe_test->tr,
+ "kprobes",
+ "gen_kretprobe_test", false));
+ trace_put_event_file(gen_kretprobe_test);
+ }
+ WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
+ }
+
+ return ret;
+}
+
+static void __exit kprobe_event_gen_test_exit(void)
+{
+ if (trace_event_file_is_valid(gen_kprobe_test)) {
+ /* Disable the event or you can't remove it */
+ WARN_ON(trace_array_set_clr_event(gen_kprobe_test->tr,
+ "kprobes",
+ "gen_kprobe_test", false));
+
+ /* Now give the file and instance back */
+ trace_put_event_file(gen_kprobe_test);
+ }
+
+
+ /* Now unregister and free the event */
+ WARN_ON(kprobe_event_delete("gen_kprobe_test"));
+
+ if (trace_event_file_is_valid(gen_kretprobe_test)) {
+ /* Disable the event or you can't remove it */
+ WARN_ON(trace_array_set_clr_event(gen_kretprobe_test->tr,
+ "kprobes",
+ "gen_kretprobe_test", false));
+
+ /* Now give the file and instance back */
+ trace_put_event_file(gen_kretprobe_test);
+ }
+
+
+ /* Now unregister and free the event */
+ WARN_ON(kprobe_event_delete("gen_kretprobe_test"));
+}
+
+module_init(kprobe_event_gen_test_init)
+module_exit(kprobe_event_gen_test_exit)
+
+MODULE_AUTHOR("Tom Zanussi");
+MODULE_DESCRIPTION("kprobe event generation test");
+MODULE_LICENSE("GPL v2");
--
2.50.1.windows.1
^ permalink raw reply related
* [PATCH] tracing: Fix multiple issues in trace_printk module handling
From: Fidal Palamparambil @ 2025-09-06 13:41 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
This commit addresses several bugs and potential issues in the
trace_printk module format handling code:
1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
proper cleanup when format string allocation fails by setting
tb_fmt to NULL after freeing it.
2. NULL pointer dereference prevention: Added NULL checks in
t_show() function before dereferencing format pointers.
3. Input validation: Added NULL check in trace_is_tracepoint_string()
to prevent potential NULL pointer dereference.
4. Type safety: Fixed type casting in t_show() to use proper
unsigned long casting for pointer arithmetic.
5. Error handling: Improved error checking in
init_trace_printk_function_export() by using IS_ERR() to check
dentry pointer.
6. Code robustness: Added additional pointer validation throughout
the code to handle potential edge cases.
7. Memory safety: Ensured consistent handling of format pointers
when memory allocation failures occur.
These fixes improve the stability and reliability of the trace_printk
infrastructure, particularly when dealing with module loading/unloading
and format string management.
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
kernel/trace/trace_printk.c | 33 +--
kernel/trace/trace_printk.c.orig | 400 +++++++++++++++++++++++++++++++
2 files changed, 419 insertions(+), 14 deletions(-)
create mode 100644 kernel/trace/trace_printk.c.orig
diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index 29f6e95439b6..cb962c6c02f8 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -76,10 +76,12 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
strcpy(fmt, *iter);
tb_fmt->fmt = fmt;
- } else
+ } else {
kfree(tb_fmt);
+ tb_fmt = NULL;
+ }
}
- *iter = fmt;
+ *iter = tb_fmt ? tb_fmt->fmt : NULL;
}
mutex_unlock(&btrace_mutex);
@@ -253,7 +255,10 @@ EXPORT_SYMBOL_GPL(__ftrace_vprintk);
bool trace_is_tracepoint_string(const char *str)
{
- const char **ptr = __start___tracepoint_str;
+ const char **ptr;
+
+ if (!str)
+ return false;
for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
if (str == *ptr)
@@ -311,19 +316,19 @@ static void *t_next(struct seq_file *m, void * v, loff_t *pos)
static int t_show(struct seq_file *m, void *v)
{
const char **fmt = v;
- const char *str = *fmt;
- int i;
+ const char *str;
- if (!*fmt)
+ if (!fmt || !*fmt)
return 0;
- seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
+ str = *fmt;
+ seq_printf(m, "0x%lx : \"", (unsigned long)fmt);
/*
* Tabs and new lines need to be converted.
*/
- for (i = 0; str[i]; i++) {
- switch (str[i]) {
+ for (; *str; str++) {
+ switch (*str) {
case '\n':
seq_puts(m, "\\n");
break;
@@ -337,7 +342,7 @@ static int t_show(struct seq_file *m, void *v)
seq_puts(m, "\\\"");
break;
default:
- seq_putc(m, str[i]);
+ seq_putc(m, *str);
}
}
seq_puts(m, "\"\n");
@@ -378,10 +383,10 @@ static const struct file_operations ftrace_formats_fops = {
static __init int init_trace_printk_function_export(void)
{
- int ret;
+ struct dentry *dentry;
- ret = tracing_init_dentry();
- if (ret)
+ dentry = tracing_init_dentry();
+ if (IS_ERR(dentry))
return 0;
trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
@@ -397,4 +402,4 @@ static __init int init_trace_printk(void)
return register_module_notifier(&module_trace_bprintk_format_nb);
}
-early_initcall(init_trace_printk);
+early_initcall(init_trace_printk);
\ No newline at end of file
diff --git a/kernel/trace/trace_printk.c.orig b/kernel/trace/trace_printk.c.orig
new file mode 100644
index 000000000000..29f6e95439b6
--- /dev/null
+++ b/kernel/trace/trace_printk.c.orig
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * trace binary printk
+ *
+ * Copyright (C) 2008 Lai Jiangshan <laijs@cn.fujitsu.com>
+ *
+ */
+#include <linux/seq_file.h>
+#include <linux/security.h>
+#include <linux/uaccess.h>
+#include <linux/kernel.h>
+#include <linux/ftrace.h>
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/ctype.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include "trace.h"
+
+#ifdef CONFIG_MODULES
+
+/*
+ * modules trace_printk()'s formats are autosaved in struct trace_bprintk_fmt
+ * which are queued on trace_bprintk_fmt_list.
+ */
+static LIST_HEAD(trace_bprintk_fmt_list);
+
+/* serialize accesses to trace_bprintk_fmt_list */
+static DEFINE_MUTEX(btrace_mutex);
+
+struct trace_bprintk_fmt {
+ struct list_head list;
+ const char *fmt;
+};
+
+static inline struct trace_bprintk_fmt *lookup_format(const char *fmt)
+{
+ struct trace_bprintk_fmt *pos;
+
+ if (!fmt)
+ return ERR_PTR(-EINVAL);
+
+ list_for_each_entry(pos, &trace_bprintk_fmt_list, list) {
+ if (!strcmp(pos->fmt, fmt))
+ return pos;
+ }
+ return NULL;
+}
+
+static
+void hold_module_trace_bprintk_format(const char **start, const char **end)
+{
+ const char **iter;
+ char *fmt;
+
+ /* allocate the trace_printk per cpu buffers */
+ if (start != end)
+ trace_printk_init_buffers();
+
+ mutex_lock(&btrace_mutex);
+ for (iter = start; iter < end; iter++) {
+ struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter);
+ if (tb_fmt) {
+ if (!IS_ERR(tb_fmt))
+ *iter = tb_fmt->fmt;
+ continue;
+ }
+
+ fmt = NULL;
+ tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL);
+ if (tb_fmt) {
+ fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL);
+ if (fmt) {
+ list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
+ strcpy(fmt, *iter);
+ tb_fmt->fmt = fmt;
+ } else
+ kfree(tb_fmt);
+ }
+ *iter = fmt;
+
+ }
+ mutex_unlock(&btrace_mutex);
+}
+
+static int module_trace_bprintk_format_notify(struct notifier_block *self,
+ unsigned long val, void *data)
+{
+ struct module *mod = data;
+ if (mod->num_trace_bprintk_fmt) {
+ const char **start = mod->trace_bprintk_fmt_start;
+ const char **end = start + mod->num_trace_bprintk_fmt;
+
+ if (val == MODULE_STATE_COMING)
+ hold_module_trace_bprintk_format(start, end);
+ }
+ return NOTIFY_OK;
+}
+
+/*
+ * The debugfs/tracing/printk_formats file maps the addresses with
+ * the ASCII formats that are used in the bprintk events in the
+ * buffer. For userspace tools to be able to decode the events from
+ * the buffer, they need to be able to map the address with the format.
+ *
+ * The addresses of the bprintk formats are in their own section
+ * __trace_printk_fmt. But for modules we copy them into a link list.
+ * The code to print the formats and their addresses passes around the
+ * address of the fmt string. If the fmt address passed into the seq
+ * functions is within the kernel core __trace_printk_fmt section, then
+ * it simply uses the next pointer in the list.
+ *
+ * When the fmt pointer is outside the kernel core __trace_printk_fmt
+ * section, then we need to read the link list pointers. The trick is
+ * we pass the address of the string to the seq function just like
+ * we do for the kernel core formats. To get back the structure that
+ * holds the format, we simply use container_of() and then go to the
+ * next format in the list.
+ */
+static const char **
+find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+{
+ struct trace_bprintk_fmt *mod_fmt;
+
+ if (list_empty(&trace_bprintk_fmt_list))
+ return NULL;
+
+ /*
+ * v will point to the address of the fmt record from t_next
+ * v will be NULL from t_start.
+ * If this is the first pointer or called from start
+ * then we need to walk the list.
+ */
+ if (!v || start_index == *pos) {
+ struct trace_bprintk_fmt *p;
+
+ /* search the module list */
+ list_for_each_entry(p, &trace_bprintk_fmt_list, list) {
+ if (start_index == *pos)
+ return &p->fmt;
+ start_index++;
+ }
+ /* pos > index */
+ return NULL;
+ }
+
+ /*
+ * v points to the address of the fmt field in the mod list
+ * structure that holds the module print format.
+ */
+ mod_fmt = container_of(v, typeof(*mod_fmt), fmt);
+ if (mod_fmt->list.next == &trace_bprintk_fmt_list)
+ return NULL;
+
+ mod_fmt = container_of(mod_fmt->list.next, typeof(*mod_fmt), list);
+
+ return &mod_fmt->fmt;
+}
+
+static void format_mod_start(void)
+{
+ mutex_lock(&btrace_mutex);
+}
+
+static void format_mod_stop(void)
+{
+ mutex_unlock(&btrace_mutex);
+}
+
+#else /* !CONFIG_MODULES */
+__init static int
+module_trace_bprintk_format_notify(struct notifier_block *self,
+ unsigned long val, void *data)
+{
+ return NOTIFY_OK;
+}
+static inline const char **
+find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+{
+ return NULL;
+}
+static inline void format_mod_start(void) { }
+static inline void format_mod_stop(void) { }
+#endif /* CONFIG_MODULES */
+
+static bool __read_mostly trace_printk_enabled = true;
+
+void trace_printk_control(bool enabled)
+{
+ trace_printk_enabled = enabled;
+}
+
+__initdata_or_module static
+struct notifier_block module_trace_bprintk_format_nb = {
+ .notifier_call = module_trace_bprintk_format_notify,
+};
+
+int __trace_bprintk(unsigned long ip, const char *fmt, ...)
+{
+ int ret;
+ va_list ap;
+
+ if (unlikely(!fmt))
+ return 0;
+
+ if (!trace_printk_enabled)
+ return 0;
+
+ va_start(ap, fmt);
+ ret = trace_vbprintk(ip, fmt, ap);
+ va_end(ap);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__trace_bprintk);
+
+int __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap)
+{
+ if (unlikely(!fmt))
+ return 0;
+
+ if (!trace_printk_enabled)
+ return 0;
+
+ return trace_vbprintk(ip, fmt, ap);
+}
+EXPORT_SYMBOL_GPL(__ftrace_vbprintk);
+
+int __trace_printk(unsigned long ip, const char *fmt, ...)
+{
+ int ret;
+ va_list ap;
+
+ if (!trace_printk_enabled)
+ return 0;
+
+ va_start(ap, fmt);
+ ret = trace_vprintk(ip, fmt, ap);
+ va_end(ap);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__trace_printk);
+
+int __ftrace_vprintk(unsigned long ip, const char *fmt, va_list ap)
+{
+ if (!trace_printk_enabled)
+ return 0;
+
+ return trace_vprintk(ip, fmt, ap);
+}
+EXPORT_SYMBOL_GPL(__ftrace_vprintk);
+
+bool trace_is_tracepoint_string(const char *str)
+{
+ const char **ptr = __start___tracepoint_str;
+
+ for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
+ if (str == *ptr)
+ return true;
+ }
+ return false;
+}
+
+static const char **find_next(void *v, loff_t *pos)
+{
+ const char **fmt = v;
+ int start_index;
+ int last_index;
+
+ start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
+
+ if (*pos < start_index)
+ return __start___trace_bprintk_fmt + *pos;
+
+ /*
+ * The __tracepoint_str section is treated the same as the
+ * __trace_printk_fmt section. The difference is that the
+ * __trace_printk_fmt section should only be used by trace_printk()
+ * in a debugging environment, as if anything exists in that section
+ * the trace_prink() helper buffers are allocated, which would just
+ * waste space in a production environment.
+ *
+ * The __tracepoint_str sections on the other hand are used by
+ * tracepoints which need to map pointers to their strings to
+ * the ASCII text for userspace.
+ */
+ last_index = start_index;
+ start_index = __stop___tracepoint_str - __start___tracepoint_str;
+
+ if (*pos < last_index + start_index)
+ return __start___tracepoint_str + (*pos - last_index);
+
+ start_index += last_index;
+ return find_next_mod_format(start_index, v, fmt, pos);
+}
+
+static void *
+t_start(struct seq_file *m, loff_t *pos)
+{
+ format_mod_start();
+ return find_next(NULL, pos);
+}
+
+static void *t_next(struct seq_file *m, void * v, loff_t *pos)
+{
+ (*pos)++;
+ return find_next(v, pos);
+}
+
+static int t_show(struct seq_file *m, void *v)
+{
+ const char **fmt = v;
+ const char *str = *fmt;
+ int i;
+
+ if (!*fmt)
+ return 0;
+
+ seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
+
+ /*
+ * Tabs and new lines need to be converted.
+ */
+ for (i = 0; str[i]; i++) {
+ switch (str[i]) {
+ case '\n':
+ seq_puts(m, "\\n");
+ break;
+ case '\t':
+ seq_puts(m, "\\t");
+ break;
+ case '\\':
+ seq_putc(m, '\\');
+ break;
+ case '"':
+ seq_puts(m, "\\\"");
+ break;
+ default:
+ seq_putc(m, str[i]);
+ }
+ }
+ seq_puts(m, "\"\n");
+
+ return 0;
+}
+
+static void t_stop(struct seq_file *m, void *p)
+{
+ format_mod_stop();
+}
+
+static const struct seq_operations show_format_seq_ops = {
+ .start = t_start,
+ .next = t_next,
+ .show = t_show,
+ .stop = t_stop,
+};
+
+static int
+ftrace_formats_open(struct inode *inode, struct file *file)
+{
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_TRACEFS);
+ if (ret)
+ return ret;
+
+ return seq_open(file, &show_format_seq_ops);
+}
+
+static const struct file_operations ftrace_formats_fops = {
+ .open = ftrace_formats_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release,
+};
+
+static __init int init_trace_printk_function_export(void)
+{
+ int ret;
+
+ ret = tracing_init_dentry();
+ if (ret)
+ return 0;
+
+ trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
+ NULL, &ftrace_formats_fops);
+
+ return 0;
+}
+
+fs_initcall(init_trace_printk_function_export);
+
+static __init int init_trace_printk(void)
+{
+ return register_module_notifier(&module_trace_bprintk_format_nb);
+}
+
+early_initcall(init_trace_printk);
--
2.50.1.windows.1
^ permalink raw reply related
* Re: [PATCH] kprobe_event_gen_test : Fix error handling and resource cleanup
From: Markus Elfring @ 2025-09-06 15:55 UTC (permalink / raw)
To: Fidal palamparambil, linux-modules
Cc: LKML, Daniel Gomez, Luis Chamberlain, Petr Pavlu, Sami Tolvanen
In-Reply-To: <20250906131559.200-1-rootuserhere@gmail.com>
> This commit addresses several issues …
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n94
…
> +++ b/kernel/trace/kprobe_event_gen_test.c
> @@ -129,6 +129,7 @@ static int __init test_gen_kprobe_cmd(void)
> "gen_kprobe_test");
> if (IS_ERR(gen_kprobe_test)) {
> ret = PTR_ERR(gen_kprobe_test);
> + gen_kprobe_test = NULL;
May the repetition be reduced for this assignment statement?
> goto delete;
How do you think about to use an additional label?
> }
…
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] arm64/acpi: Fix multiple issues in ACPI boot support code
From: Markus Elfring @ 2025-09-06 16:11 UTC (permalink / raw)
To: Fidal palamparambil, linux-modules
Cc: LKML, Daniel Gomez, Luis Chamberlain, Petr Pavlu, Sami Tolvanen
In-Reply-To: <20250906122731.946-1-rootuserhere@gmail.com>
…
> These fixes address potential memory leaks, compilation warnings, and
> improper error handling in the ARM64 ACPI boot support code.
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n81
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] module : fix signature checker pointer arithmetic and bounds check
From: Luis Chamberlain @ 2025-09-07 1:21 UTC (permalink / raw)
To: Fidal Palamparambil
Cc: linux-modules, petr.pavlu, da.gomez, samitolvanen, linux-kernel
In-Reply-To: <20250905154550.130-1-rootuserhere@gmail.com>
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
^ permalink raw reply
* Re: [PATCH] arm64/acpi : Fix multiple issues in ACPI boot support code
From: Luis Chamberlain @ 2025-09-07 1:21 UTC (permalink / raw)
To: Fidal Palamparambil
Cc: linux-modules, petr.pavlu, da.gomez, samitolvanen, linux-kernel
In-Reply-To: <20250906121711.1647-1-rootuserhere@gmail.com>
On Sat, Sep 06, 2025 at 04:17:11PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
>
> - Fixed memory leak in acpi_fadt_sanity_check() by ensuring acpi_put_table()
> is called in all error paths
> - Corrected error handling in parse_acpi() by removing incorrect snprintf() usage
> - Added missing compiler_attributes.h include for fallthrough support
> - Verified proper NULL pointer checks in acpi_os_ioremap()
>
> These fixes address potential memory leaks, compilation warnings, and
> improper error handling in the ARM64 ACPI boot support code.
>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
Please stop with your garbage patches. They are not welcomed and wreckless.
Luis
^ permalink raw reply
* Re: [PATCH] cgroup: debug: Fix multiple issues in debug controller implementation
From: Luis Chamberlain @ 2025-09-07 1:22 UTC (permalink / raw)
To: Fidal Palamparambil
Cc: linux-modules, petr.pavlu, da.gomez, samitolvanen, linux-kernel
In-Reply-To: <20250906124417.793-1-rootuserhere@gmail.com>
On Sat, Sep 06, 2025 at 04:44:17PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
>
> This commit addresses several issues in the cgroup debug controller:
>
> 1. Memory leak protection in current_css_set_cg_links_read():
> - Initialize name_buf to NULL and use proper error handling with goto
> - Ensure memory is freed in all code paths
>
> 2. RCU locking correctness:
> - Use lockdep_is_held() for proper lock dependency tracking in
> rcu_dereference_protected() calls
> - Ensure proper locking context is passed to RCU checks
>
> 3. Buffer size safety:
> - Remove off-by-one in snprintf() size calculation
> - Use sizeof(buffer) instead of sizeof(buffer)-1
>
> 4. Code robustness:
> - Add proper initialization of variables
> - Use consistent error handling patterns
> - Fix potential integer underflow in extra reference calculation
>
> The debug controller is for internal debugging only, but these fixes
> ensure it doesn't introduce stability issues or memory leaks when
> used for debugging purposes.
>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
We really need to just ban you.
Luis
^ permalink raw reply
* Re: [PATCH] kprobe_event_gen_test : Fix error handling and resource cleanup
From: Luis Chamberlain @ 2025-09-07 1:23 UTC (permalink / raw)
To: Fidal Palamparambil
Cc: linux-modules, petr.pavlu, da.gomez, samitolvanen, linux-kernel
In-Reply-To: <20250906131559.200-1-rootuserhere@gmail.com>
On Sat, Sep 06, 2025 at 05:15:59PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
>
> This commit addresses several issues in the kprobe event generation test module:
>
> 1. Fixed NULL pointer management in error paths where trace_event_file pointers
> were not properly set to NULL after errors, potentially causing double-free
> or use-after-free issues.
>
> 2. Added comprehensive cleanup for the kprobe event when kretprobe initialization
> fails in kprobe_event_gen_test_init(). Previously, a failed kretprobe creation
> would leave the kprobe event dangling.
>
> 3. Enhanced error handling consistency between kprobe and kretprobe test functions,
> ensuring both follow the same pattern for resource cleanup.
>
> 4. Fixed the error handling flow in test_gen_kprobe_cmd() and test_gen_kretprobe_cmd()
> to properly set trace_event_file pointers to NULL when errors occur after
> event creation but before successful completion.
>
> The changes ensure proper resource management and prevent potential memory
> corruption or leaks during module initialization and cleanup.
>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
Please just stop.
Luis
^ permalink raw reply
* Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling
From: Luis Chamberlain @ 2025-09-07 1:23 UTC (permalink / raw)
To: Fidal Palamparambil
Cc: linux-modules, petr.pavlu, da.gomez, samitolvanen, linux-kernel
In-Reply-To: <20250906134148.55-1-rootuserhere@gmail.com>
On Sat, Sep 06, 2025 at 05:41:48PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
>
> This commit addresses several bugs and potential issues in the
> trace_printk module format handling code:
>
> 1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
> proper cleanup when format string allocation fails by setting
> tb_fmt to NULL after freeing it.
>
> 2. NULL pointer dereference prevention: Added NULL checks in
> t_show() function before dereferencing format pointers.
>
> 3. Input validation: Added NULL check in trace_is_tracepoint_string()
> to prevent potential NULL pointer dereference.
>
> 4. Type safety: Fixed type casting in t_show() to use proper
> unsigned long casting for pointer arithmetic.
>
> 5. Error handling: Improved error checking in
> init_trace_printk_function_export() by using IS_ERR() to check
> dentry pointer.
>
> 6. Code robustness: Added additional pointer validation throughout
> the code to handle potential edge cases.
>
> 7. Memory safety: Ensured consistent handling of format pointers
> when memory allocation failures occur.
>
> These fixes improve the stability and reliability of the trace_printk
> infrastructure, particularly when dealing with module loading/unloading
> and format string management.
>
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
Anothe waste of time. Stop. I'll suggest everyone ignore your patches.
Luis
^ permalink raw reply
* Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling
From: kernel test robot @ 2025-09-07 1:56 UTC (permalink / raw)
To: Fidal Palamparambil, linux-modules
Cc: oe-kbuild-all, mcgrof, petr.pavlu, da.gomez, samitolvanen,
linux-kernel, Fidal palamparambil
In-Reply-To: <20250906134148.55-1-rootuserhere@gmail.com>
Hi Fidal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: x86_64-buildonly-randconfig-001-20250907 (https://download.01.org/0day-ci/archive/20250907/202509070951.9KQcO9l6-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250907/202509070951.9KQcO9l6-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509070951.9KQcO9l6-lkp@intel.com/
All warnings (new ones prefixed by >>):
kernel/trace/trace_printk.c: In function 'init_trace_printk_function_export':
>> kernel/trace/trace_printk.c:388:16: warning: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
388 | dentry = tracing_init_dentry();
| ^
vim +388 kernel/trace/trace_printk.c
383
384 static __init int init_trace_printk_function_export(void)
385 {
386 struct dentry *dentry;
387
> 388 dentry = tracing_init_dentry();
389 if (IS_ERR(dentry))
390 return 0;
391
392 trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
393 NULL, &ftrace_formats_fops);
394
395 return 0;
396 }
397
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling
From: kernel test robot @ 2025-09-07 4:11 UTC (permalink / raw)
To: Fidal Palamparambil, linux-modules
Cc: oe-kbuild-all, mcgrof, petr.pavlu, da.gomez, samitolvanen,
linux-kernel, Fidal palamparambil
In-Reply-To: <20250906134148.55-1-rootuserhere@gmail.com>
Hi Fidal,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
[also build test ERROR on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: s390-randconfig-002-20250907 (https://download.01.org/0day-ci/archive/20250907/202509071126.l4plN0Qk-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250907/202509071126.l4plN0Qk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509071126.l4plN0Qk-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/trace/trace_printk.c: In function 'init_trace_printk_function_export':
>> kernel/trace/trace_printk.c:388:16: error: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
388 | dentry = tracing_init_dentry();
| ^
vim +388 kernel/trace/trace_printk.c
383
384 static __init int init_trace_printk_function_export(void)
385 {
386 struct dentry *dentry;
387
> 388 dentry = tracing_init_dentry();
389 if (IS_ERR(dentry))
390 return 0;
391
392 trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
393 NULL, &ftrace_formats_fops);
394
395 return 0;
396 }
397
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling
From: kernel test robot @ 2025-09-07 7:17 UTC (permalink / raw)
To: Fidal Palamparambil, linux-modules
Cc: oe-kbuild-all, mcgrof, petr.pavlu, da.gomez, samitolvanen,
linux-kernel, Fidal palamparambil
In-Reply-To: <20250906134148.55-1-rootuserhere@gmail.com>
Hi Fidal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: powerpc-randconfig-r131-20250907 (https://download.01.org/0day-ci/archive/20250907/202509071540.GTxwwstz-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 13.4.0
reproduce: (https://download.01.org/0day-ci/archive/20250907/202509071540.GTxwwstz-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509071540.GTxwwstz-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> kernel/trace/trace_printk.c:388:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected struct dentry *dentry @@ got int @@
kernel/trace/trace_printk.c:388:16: sparse: expected struct dentry *dentry
kernel/trace/trace_printk.c:388:16: sparse: got int
vim +388 kernel/trace/trace_printk.c
383
384 static __init int init_trace_printk_function_export(void)
385 {
386 struct dentry *dentry;
387
> 388 dentry = tracing_init_dentry();
389 if (IS_ERR(dentry))
390 return 0;
391
392 trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
393 NULL, &ftrace_formats_fops);
394
395 return 0;
396 }
397
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] tracing : Fix multiple issues in trace_printk module handling
From: Fidal Palamparambil @ 2025-09-07 13:52 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
This commit addresses several bugs and potential issues in the
trace_printk module format handling code:
1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
proper cleanup when format string allocation fails by setting
tb_fmt to NULL after freeing it to prevent memory leaks.
2. NULL pointer dereference prevention: Added comprehensive NULL checks
in t_show() function before dereferencing format pointers to prevent
kernel crashes.
3. Input validation: Added NULL check in trace_is_tracepoint_string()
to prevent potential NULL pointer dereference when called with
invalid input.
4. Type safety: Fixed type casting in t_show() to use proper
unsigned long casting for pointer arithmetic, ensuring correct
pointer handling across different architectures.
5. Error handling: Fixed type mismatch in init_trace_printk_function_export()
by properly handling struct dentry pointer return from tracing_init_dentry()
and using IS_ERR_OR_NULL() for comprehensive error checking.
6. Code robustness: Added additional pointer validation throughout
the code to handle potential edge cases and improve overall stability.
7. Memory safety: Ensured consistent handling of format pointers
when memory allocation failures occur, preventing use-after-free
and other memory corruption issues.
These fixes improve the stability and reliability of the trace_printk
infrastructure, particularly when dealing with module loading/unloading
and format string management.
Reported-by : kernel test robot <lkp@intel.com>
Closes : https://lore.kernel.org/oe-kbuild-all/202509071540.GTxwwstz-lkp@intel.com/
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
kernel/trace/trace_printk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index cb962c6c02f8..665effbf50ae 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -386,7 +386,7 @@ static __init int init_trace_printk_function_export(void)
struct dentry *dentry;
dentry = tracing_init_dentry();
- if (IS_ERR(dentry))
+ if (IS_ERR_OR_NULL(dentry))
return 0;
trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
--
2.50.1.windows.1
^ permalink raw reply related
* [PATCH] Fixed the build warning in init_trace_printk_function_export():
From: Fidal Palamparambil @ 2025-09-07 14:07 UTC (permalink / raw)
To: linux-modules
Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, linux-kernel,
Fidal palamparambil
From: Fidal palamparambil <rootuserhere@gmail.com>
Changed int ret to struct dentry *dentry
Changed if (ret) to if (IS_ERR_OR_NULL(dentry))
Fixed memory leak in hold_module_trace_bprintk_format():
Added proper cleanup when fmt allocation fails
Set tb_fmt = NULL after freeing to prevent dangling pointers
Fixed NULL pointer dereference in t_show():
Added if (!fmt || !*fmt) check before dereferencing
Simplified the string iteration loop
Added NULL check in trace_is_tracepoint_string():
Added if (!str) check to prevent NULL pointer dereference
Fixed type safety in t_show():
Changed *(unsigned long *)fmt to (unsigned long)fmt for correct pointer casting
Fixed function signature in ftrace_formats_open():
Changed struct file *file to const struct file *file for consistency
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
kernel/trace/trace_printk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index 665effbf50ae..060bd2c35a7d 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -363,7 +363,7 @@ static const struct seq_operations show_format_seq_ops = {
};
static int
-ftrace_formats_open(struct inode *inode, struct file *file)
+ftrace_formats_open(struct inode *inode, const struct file *file)
{
int ret;
--
2.50.1.windows.1
^ permalink raw reply related
* Re: [PATCH] Fixed the build warning in init_trace_printk_function_export():
From: kernel test robot @ 2025-09-07 17:01 UTC (permalink / raw)
To: Fidal Palamparambil, linux-modules
Cc: oe-kbuild-all, mcgrof, petr.pavlu, da.gomez, samitolvanen,
linux-kernel, Fidal palamparambil
In-Reply-To: <20250907140755.529-1-rootuserhere@gmail.com>
Hi Fidal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/Fixed-the-build-warning-in-init_trace_printk_function_export/20250907-221041
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250907140755.529-1-rootuserhere%40gmail.com
patch subject: [PATCH] Fixed the build warning in init_trace_printk_function_export():
config: x86_64-buildonly-randconfig-001-20250907 (https://download.01.org/0day-ci/archive/20250908/202509080040.8gEyq9Ef-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250908/202509080040.8gEyq9Ef-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509080040.8gEyq9Ef-lkp@intel.com/
All warnings (new ones prefixed by >>):
kernel/trace/trace_printk.c: In function 'ftrace_formats_open':
>> kernel/trace/trace_printk.c:369:25: warning: passing argument 1 of 'seq_open' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
369 | return seq_open(file, &show_format_seq_ops);
| ^~~~
In file included from kernel/trace/trace_printk.c:8:
include/linux/seq_file.h:108:14: note: expected 'struct file *' but argument is of type 'const struct file *'
108 | int seq_open(struct file *, const struct seq_operations *);
| ^~~~~~~~~~~~~
kernel/trace/trace_printk.c: At top level:
kernel/trace/trace_printk.c:373:17: error: initialization of 'int (*)(struct inode *, struct file *)' from incompatible pointer type 'int (*)(struct inode *, const struct file *)' [-Werror=incompatible-pointer-types]
373 | .open = ftrace_formats_open,
| ^~~~~~~~~~~~~~~~~~~
kernel/trace/trace_printk.c:373:17: note: (near initialization for 'ftrace_formats_fops.open')
cc1: some warnings being treated as errors
vim +369 kernel/trace/trace_printk.c
7975a2be16dd42 Steven Rostedt 2009-03-12 359
7975a2be16dd42 Steven Rostedt 2009-03-12 360 static int
66670b02cb828c Fidal palamparambil 2025-09-07 361 ftrace_formats_open(struct inode *inode, const struct file *file)
7975a2be16dd42 Steven Rostedt 2009-03-12 362 {
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 363) int ret;
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 364)
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 365) ret = security_locked_down(LOCKDOWN_TRACEFS);
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 366) if (ret)
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 367) return ret;
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 368)
c8961ec6da22ea Li Zefan 2009-06-24 @369 return seq_open(file, &show_format_seq_ops);
7975a2be16dd42 Steven Rostedt 2009-03-12 370 }
7975a2be16dd42 Steven Rostedt 2009-03-12 371
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] Fixed the build warning in init_trace_printk_function_export():
From: kernel test robot @ 2025-09-07 18:33 UTC (permalink / raw)
To: Fidal Palamparambil, linux-modules
Cc: llvm, oe-kbuild-all, mcgrof, petr.pavlu, da.gomez, samitolvanen,
linux-kernel, Fidal palamparambil
In-Reply-To: <20250907140755.529-1-rootuserhere@gmail.com>
Hi Fidal,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
[also build test ERROR on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/Fixed-the-build-warning-in-init_trace_printk_function_export/20250907-221041
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250907140755.529-1-rootuserhere%40gmail.com
patch subject: [PATCH] Fixed the build warning in init_trace_printk_function_export():
config: x86_64-buildonly-randconfig-002-20250907 (https://download.01.org/0day-ci/archive/20250908/202509080203.SxCdQOOY-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250908/202509080203.SxCdQOOY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509080203.SxCdQOOY-lkp@intel.com/
All errors (new ones prefixed by >>):
>> kernel/trace/trace_printk.c:369:18: error: passing 'const struct file *' to parameter of type 'struct file *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
369 | return seq_open(file, &show_format_seq_ops);
| ^~~~
include/linux/seq_file.h:108:27: note: passing argument to parameter here
108 | int seq_open(struct file *, const struct seq_operations *);
| ^
>> kernel/trace/trace_printk.c:373:10: error: incompatible function pointer types initializing 'int (*)(struct inode *, struct file *)' with an expression of type 'int (struct inode *, const struct file *)' [-Wincompatible-function-pointer-types]
373 | .open = ftrace_formats_open,
| ^~~~~~~~~~~~~~~~~~~
2 errors generated.
vim +369 kernel/trace/trace_printk.c
7975a2be16dd42 Steven Rostedt 2009-03-12 359
7975a2be16dd42 Steven Rostedt 2009-03-12 360 static int
66670b02cb828c Fidal palamparambil 2025-09-07 361 ftrace_formats_open(struct inode *inode, const struct file *file)
7975a2be16dd42 Steven Rostedt 2009-03-12 362 {
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 363) int ret;
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 364)
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 365) ret = security_locked_down(LOCKDOWN_TRACEFS);
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 366) if (ret)
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 367) return ret;
17911ff38aa58d Steven Rostedt (VMware 2019-10-11 368)
c8961ec6da22ea Li Zefan 2009-06-24 @369 return seq_open(file, &show_format_seq_ops);
7975a2be16dd42 Steven Rostedt 2009-03-12 370 }
7975a2be16dd42 Steven Rostedt 2009-03-12 371
7975a2be16dd42 Steven Rostedt 2009-03-12 372 static const struct file_operations ftrace_formats_fops = {
7975a2be16dd42 Steven Rostedt 2009-03-12 @373 .open = ftrace_formats_open,
7975a2be16dd42 Steven Rostedt 2009-03-12 374 .read = seq_read,
7975a2be16dd42 Steven Rostedt 2009-03-12 375 .llseek = seq_lseek,
7975a2be16dd42 Steven Rostedt 2009-03-12 376 .release = seq_release,
7975a2be16dd42 Steven Rostedt 2009-03-12 377 };
7975a2be16dd42 Steven Rostedt 2009-03-12 378
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox