linux-kernel - Re: [PATCH] printk: Userspace format enumeration support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 06 Feb 2021 09:57:00 -0800
From:   Joe Perches <joe@...ches.com>
To:     Chris Down <chris@...isdown.name>, Petr Mladek <pmladek@...e.com>
Cc:     linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        John Ogness <john.ogness@...utronix.de>,
        Johannes Weiner <hannes@...xchg.org>,
        Andrew Morton <akpm@...ux-foundation.org>, kernel-team@...com,
        Steven Rostedt <rostedt@...dmis.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jason Baron <jbaron@...mai.com>,
        Kees Cook <keescook@...omium.org>, linux-api@...r.kernel.org
Subject: Re: [PATCH] printk: Userspace format enumeration support

On Fri, 2021-02-05 at 22:25 +0000, Chris Down wrote:
> Petr Mladek writes:
> >   + <module> is already optinaly added by pr_fmt() to the printed strings
> >     as:  pr_fmt(): ...
> 
> pr_fmts are not consistently used across the kernel, and sometimes differ from 
> the module itself. Many modules don't use it at all, and we also don't have it 
> for pr_cont. Just picking some random examples:
> 
>      % grep -av vmlinux /proc/printk_formats | shuf -n 10
>      mac80211,6%s: mesh STA %pM switches to channel requiring DFS (%d MHz, width:%d, CF1/2: %d/%d MHz), aborting
>      thinkpad_acpi,c N/Athinkpad_acpi,c %dthinkpad_acpi,5thinkpad_acpi: temperatures (Celsius):thinkpad_acpi,3thinkpad_acpi: Out of memory for LED data

I don't understand this format.

"Out of memory for LED data" is a single printk ending with a '\n' newline 
I expected this to be broken up into multiple lines, one for each printk
that endsd in a newline.

And what would happen if the function was refactored removing the pr_cont
uses like the below: (basically, any output that uses a mechanism that
aggregates a buffer then emits it, and there are a _lot_ of those)

	printk("%s\n", buffer);

And there is already a relatively trivial way to do this using a modified
version of strings that looks for KERN_SOH[0-6], and if dynamic_debug is
enabled, look in the dynamic_debug section, either __verbose or __dyndbg
depending on the kernel version.

---
 drivers/platform/x86/thinkpad_acpi.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 18b390153e7f..ff1c09c600f8 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6353,21 +6353,26 @@ static void thermal_dump_all_sensors(void)
 {
 	int n, i;
 	struct ibm_thermal_sensors_struct t;
+	char output[256];
+	int len = 0;
 
 	n = thermal_get_sensors(&t);
 	if (n <= 0)
 		return;
 
-	pr_notice("temperatures (Celsius):");
+	len += scnprintf(output + len, sizeof(output) - len,
+		       "temperatures (Celsius):");
 
 	for (i = 0; i < n; i++) {
-		if (t.temp[i] != TPACPI_THERMAL_SENSOR_NA)
-			pr_cont(" %d", (int)(t.temp[i] / 1000));
+		if (t.temp[i] == TPACPI_THERMAL_SENSOR_NA)
+			len += scnprintf(output + len, sizeof(output) - len,
+					 " N/A");
 		else
-			pr_cont(" N/A");
+			len += scnprintf(output + len, sizeof(output) - len,
+					 " %d", t.temp[i] / 1000);
 	}
 
-	pr_cont("\n");
+	pr_notice("%s\n", output);
 }
 
 /* sysfs temp##_input -------------------------------------------------- */