printk: Userspace format indexing support

We have a number of systems industry-wide that have a subset of their functionality that works as follows:

1. Receive a message from local kmsg, serial console, or netconsole;
2. Apply a set of rules to classify the message;
3. Do something based on this classification (like scheduling a remediation for the machine), rinse, and repeat.

As a couple of examples of places we have this implemented just inside Facebook, although this isn't a Facebook-specific problem, we have this inside our netconsole processing (for alarm classification), and as part of our machine health checking. We use these messages to determine fairly important metrics around production health, and it's important that we get them right.

While for some kinds of issues we have counters, tracepoints, or metrics with a stable interface which can reliably indicate the issue, in order to react to production issues quickly we need to work with the interface which most kernel developers naturally use when developing: printk.

Most production issues come from unexpected phenomena, and as such usually the code in question doesn't have easily usable tracepoints or other counters available for the specific problem being mitigated. We have a number of lines of monitoring defence against problems in production (host metrics, process metrics, service metrics, etc), and where it's not feasible to reliably monitor at another level, this kind of pragmatic netconsole monitoring is essential.

As one would expect, monitoring using printk is rather brittle for a number of reasons -- most notably that the message might disappear entirely in a new version of the kernel, or that the message may change in some way that the regex or other classification methods start to silently fail.

One factor that makes this even harder is that, under normal operation, many of these messages are never expected to be hit. For example, there may be a rare hardware bug which one wants to detect if it was to ever happen again, but its recurrence is not likely or anticipated. This precludes using something like checking whether the printk in question was printed somewhere fleetwide recently to determine whether the message in question is still present or not, since we don't anticipate that it should be printed anywhere, but still need to monitor for its future presence in the long-term.

This class of issue has happened on a number of occasions, causing unhealthy machines with hardware issues to remain in production for longer than ideal. As a recent example, some monitoring around blk_update_request fell out of date and caused semi-broken machines to remain in production for longer than would be desirable.

Searching through the codebase to find the message is also extremely fragile, because many of the messages are further constructed beyond their callsite (eg. btrfs_printk and other module-specific wrappers, each with their own functionality). Even if they aren't, guessing the format and formulation of the underlying message based on the aesthetics of the message emitted is not a recipe for success at scale, and our previous issues with fleetwide machine health checking demonstrate as much.

This provides a solution to the issue of silently changed or deleted
printks: we record pointers to all printk format strings known at compile time into a new .printk_index section, both in vmlinux and modules. At runtime, this can then be iterated by looking at <debugfs>/printk/index/<module>, which emits the following format, both readable by humans and able to be parsed by machines:

    $ head -1 vmlinux; shuf -n 5 vmlinux
    # <level[,flags]> filename:line function "format"
    <5> block/blk-settings.c:661 disk_stack_limits "%s: Warning: Device %s is misaligned\n"
    <4> kernel/trace/trace.c:8296 trace_create_file "Could not create tracefs '%s' entry\n"
    <6> arch/x86/kernel/hpet.c:144 _hpet_print_config "hpet: %s(%d):\n"
    <6> init/do_mounts.c:605 prepare_namespace "Waiting for root device %s...\n"
    <6> drivers/acpi/osl.c:1410 acpi_no_auto_serialize_setup "ACPI: auto-serialization disabled\n"

This mitigates the majority of cases where we have a highly-specific printk which we want to match on, as we can now enumerate and check whether the format changed or the printk callsite disappeared entirely in userspace. This allows us to catch changes to printks we monitor earlier and decide what to do about it before it becomes problematic.

There is no additional runtime cost for printk callers or printk itself, and the assembly generated is exactly the same.

---

v2:

- Use seq_printf instead of step by step accumulation
- Scope fptr closer to its use
- Prevent seq_file from needing to alloc a new buffer
- Always provide KERN_SOH + level, even if caller omitted it
- Provide one file per module
- Update changelog to show ease of parsing
- Provide printk -> _printk for ARCH=um (thanks, LKP robot)
- Move to debugfs (thanks, Steven)

---

v3:

- Reduce locking during open by moving size lifetime to the inode
- Don't explicitly check for debugfs errors (thanks, gregkh)

---

v4:

- Fix a missed extern int printk fixup in ia64 (thanks, lkp)

---

v5:

Thanks Petr for the extensive feedback.

- Move all module handling from module notifier to module.[ch] directly
- Change to readable output format
- Handled by new string_escape(ESCAPE_PRINTF)
- Add file/line/function information
- Mass rename everything to printk_index/CONFIG_PRINTK_INDEX/pi_*
- As a result, this version does away with the mutex/hashtable
- Use seq_file iteration API instead of simple_open
- Remove debugfs file first to avoid ENOENT
- Tear down backing datastructure if debugfs file creation fails
- Move code under ifdef guard to kernel/printk/index.c
- Add pi_sec (formerly printk_fmt_sec) documentation
- Handle coexisting LOG_CONT + level
- Add header to debugfs output
- ...and probably some other stuff I forgot. :-)

---

v6:

- Rebase on next-20210518
- Remove pi_sec, do on demand lookups instead
- Lookup debugfs file on demand
- pi_sec_{store,remove} now only changes debugfs, so renamed
- Don't check for debugfs failure (again)
- Extract lookup logic into pi_get_entry
- Make the stored struct const __used
- Use _p_func directly, which allows including void-returning functions
- Move more stuff outside PRINTK_INDEX #ifdef using __printk_index_emit
- Scope __{start,stop}_printk_index more tightly
- Move parse_prefix to printk_parse_prefix in internal.h
- Put PRINTK_INDEX definition below TRACEDATA
- Use a module notifier again
- pi_object -> pi_entry, with clearer vars
- Reword Kconfig
- Split out printk_info_flags/string_helpers stuff into patches
- Use seq_escape_str directly now that ESCAPE_APPEND exists
- Add dev_printk indexing support

---

v7:

- Rebase on next-20210615
- Change some GCC statement expressions where do {} while (0) could work
- dev_* needs to be statement due to SCSI_LOG_ERROR_RECOVERY and similar
- Fix some weird continuation indentation
- Remove filename from index.c header
- Add explicit default case for non MODULE_STATE_COMING/MODULE_STATE_GOING
- Tersify !CONFIG_MODULES case for pi_setup_module_notifier
- Remove now unnecessary pi_start forward declaration
- Move to DEFINE_SEQ_ATTRIBUTE for dfs_index
- Add comment explaining why we have both ESCAPE_SPECIAL and subset as @only
- Remove post_fmt for now, since dev_printk doesn't have it and it looks rare
- Rename pre_fmt to subsys_fmt_prefix
- Avoid forward declarations for pi_{create,remove}_file by splitting pi_get_module_name from other CONFIG_MODULES dependents
- Don't show -1 for LOGLEVEL_DEFAULT with LOG_CONT, since it has a different semantic meaning
- Some improvements to comments based on feedback from Petr
- Split out printk_parse_prefix externalisation into a separate patch
- Move printk_parse_prefix into internal.h instead of printk_ringbuffer.h
- Now that level might be optional (eg. in LOG_CONT with no level case), update the header to match
- Reduce duplication in dev_printk

Chris Down (5):
string_helpers: Escape double quotes in escape_special
printk: Straighten out log_flags into printk_info_flags
printk: Rework parse_prefix into printk_parse_prefix
printk: Userspace format indexing support
printk: index: Add indexing support to dev_printk

 MAINTAINERS                          |   5 +
 arch/arm/kernel/entry-v7m.S          |   2 +-
 arch/arm/lib/backtrace-clang.S       |   2 +-
 arch/arm/lib/backtrace.S             |   2 +-
 arch/arm/mach-rpc/io-acorn.S         |   2 +-
 arch/arm/vfp/vfphw.S                 |   6 +-
 arch/ia64/include/uapi/asm/cmpxchg.h |   4 +-
 arch/openrisc/kernel/entry.S         |   6 +-
 arch/powerpc/kernel/head_fsl_booke.S |   2 +-
 arch/um/include/shared/user.h        |   3 +-
 arch/x86/kernel/head_32.S            |   2 +-
 drivers/base/core.c                  |   6 +-
 include/asm-generic/vmlinux.lds.h    |  13 ++
 include/linux/dev_printk.h           |  66 ++++++---
 include/linux/module.h               |   5 +
 include/linux/printk.h               |  95 ++++++++++++-
 init/Kconfig                         |  14 ++
 kernel/module.c                      |   5 +
 kernel/printk/Makefile               |   1 +
 kernel/printk/index.c                | 195 +++++++++++++++++++++++++++
 kernel/printk/internal.h             |   8 ++
 kernel/printk/printk.c               |  60 ++++-----
 lib/string_helpers.c                 |   4 +
 lib/test-string_helpers.c            |  14 +-
 24 files changed, 446 insertions(+), 76 deletions(-)
 create mode 100644 kernel/printk/index.c

2.31.1

From an abstract point of view, escape_special's counterpart, unescape_special, already handles the unescaping of blackslashed double quote sequences.

As a more practical example, printk indexing is an example case where this is already practically useful. Compare an example with ESCAPE_SPECIAL | ESCAPE_SPACE, with quotes not escaped:

[root@ktst ~]# grep drivers/pci/pci-stub.c:69 /sys/kernel/debug/printk/index/vmlinux <4> drivers/pci/pci-stub.c:69 pci_stub_init "pci-stub: invalid ID string "%s"\n"

...and the same after this patch:

[root@ktst ~]# grep drivers/pci/pci-stub.c:69 /sys/kernel/debug/printk/index/vmlinux <4> drivers/pci/pci-stub.c:69 pci_stub_init "pci-stub: invalid ID string \"%s\"\n"

One can of course, alternatively, use ESCAPE_APPEND with a quote in @only, but without this patch quotes are coerced into hex or octal which can hurt readability quite significantly.

I've checked uses of ESCAPE_SPECIAL and %pE across the codebase, and I'm pretty confident that this shouldn't affect any stable interfaces.

---
 lib/string_helpers.c      |  4 ++++
 lib/test-string_helpers.c | 14 +++++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)

index 5a35c7e16e96..3806a52ce697 100644
@@ -361,6 +361,9 @@ static bool escape_special(unsigned char c, char **dst, char *end)
 	case '\e':
 		to = 'e';
 		break;
+	case '"':
+		to = '"';
+		break;
 	default:
 		return false;
 	}

@@ -474,6 +477,7 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
  *		'\t' - horizontal tab
  *		'\v' - vertical tab
  *	%ESCAPE_SPECIAL:
+ *		'\"' - double quote
  *		'\\' - backslash
  *		'\a' - alert (BEL)
  *		'\e' - escape

index 2185d71704f0..437d8e6b7cb1 100644
@@ -140,13 +140,13 @@ static const struct test_string_2 escape0[] __initconst = {{
 },{
 	.in = "\\h\\\"\a\e\\",
 	.s1 = {{
-		.out = "\\\\h\\\\\"\\a\\e\\\\",
+		.out = "\\\\h\\\\\\\"\\a\\e\\\\",
 		.flags = ESCAPE_SPECIAL,
 	},{
-		.out = "\\\\\\150\\\\\\042\\a\\e\\\\",
+		.out = "\\\\\\150\\\\\\\"\\a\\e\\\\",
 		.flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
 	},{
-		.out = "\\\\\\x68\\\\\\x22\\a\\e\\\\",
+		.out = "\\\\\\x68\\\\\\\"\\a\\e\\\\",
 		.flags = ESCAPE_SPECIAL | ESCAPE_HEX,
 	},{
 		/* terminator */

@@ -157,10 +157,10 @@ static const struct test_string_2 escape0[] __initconst = {{
 		.out = "\eb \\C\007\"\x90\\r]",
 		.flags = ESCAPE_SPACE,
 	},{
-		.out = "\\eb \\\\C\\a\"\x90\r]",
+		.out = "\\eb \\\\C\\a\\\"\x90\r]",
 		.flags = ESCAPE_SPECIAL,
 	},{
-		.out = "\\eb \\\\C\\a\"\x90\\r]",
+		.out = "\\eb \\\\C\\a\\\"\x90\\r]",
 		.flags = ESCAPE_SPACE | ESCAPE_SPECIAL,
 	},{
 		.out = "\\033\\142\\040\\134\\103\\007\\042\\220\\015\\135",

@@ -169,10 +169,10 @@ static const struct test_string_2 escape0[] __initconst = {{
 		.out = "\\033\\142\\040\\134\\103\\007\\042\\220\\r\\135",
 		.flags = ESCAPE_SPACE | ESCAPE_OCTAL,
 	},{
-		.out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\015\\135",
+		.out = "\\e\\142\\040\\\\\\103\\a\\\"\\220\\015\\135",
 		.flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
 	},{
-		.out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\r\\135",
+		.out = "\\e\\142\\040\\\\\\103\\a\\\"\\220\\r\\135",
 		.flags = ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_OCTAL,
 	},{
 		.out = "\eb \\C\007\"\x90\r]",

2.31.1

On Tue, Jun 15, 2021 at 7:53 PM Chris Down <[email protected]> wrote:

In case you will need to send a new version some nit picks below WRT commit message. Or somebody might fix them in place when applying.

(example example)

"As a more practical example, printk indexing is the case where..."

?

In both examples: '[root@ktst ~]#' => '#'

checked the uses

With Best Regards,
Andy Shevchenko

Hmm, what's wrong with using the zero article for "checked uses"? I mean, I don't have any strong resistance, but I don't see anything wrong with it either, and it matches how I'd naturally speak.

Agreed on the others, though, hopefully they can be massaged in :-)

Ack, I don't see anything wrong with it either.

I guess you know better than me :-)

I'm just a mere non-native speaker here to whom it's stylistically harder to parse without the article.

With Best Regards,
Andy Shevchenko

English is weird. It sounds fine to me with and without the "the" article, but I'll say having "the" does make it a little smoother, and probably better for the non-native speaking folks.

Technically, I'm a native speaker, but I prefer to say my mother tongue is C. ;-)

-- Steve

In the past, enum log_flags was part of struct log, hence the name. struct log has since been reworked and now this struct is stored inside struct printk_info. However, the name was never updated, which is somewhat confusing -- especially since these flags operate at the record level rather than at the level of an abstract log.

printk_info_flags also joins its other metadata struct friends in printk_ringbuffer.h.

---
 kernel/printk/internal.h |  6 ++++++
 kernel/printk/printk.c   | 43 +++++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 23 deletions(-)

index 51615c909b2f..1075e60fcd98 100644
@@ -12,6 +12,12 @@
 
 #define PRINTK_NMI_CONTEXT_OFFSET	0x010000000
 
+/* Flags for a single printk record. */
+enum printk_info_flags {
+	LOG_NEWLINE	= 2,	/* text ended with a newline */
+	LOG_CONT	= 8,	/* text is a fragment of a continuation line */
+};
+
 __printf(4, 0)
 int vprintk_store(int facility, int level,
 		  const struct dev_printk_info *dev_info,

2.31.1

parse_prefix is needed externally by later patches, so move it into a context where it can be used as such. Also give it the printk_ prefix to reduce the chance of collisions.

---
 kernel/printk/internal.h | 2 ++
 kernel/printk/printk.c   | 8 ++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

index 1075e60fcd98..1596e2837318 100644
@@ -57,6 +57,8 @@ bool printk_percpu_data_ready(void);
 
 void defer_console_output(void);
 
+u16 printk_parse_prefix(const char *text, int *level,
+			enum printk_info_flags *flags);
 #else
 
 /*

index 3201bb0c269c..26d52aea9969 100644
@@ -1957,7 +1957,7 @@ static inline u32 printk_caller_id(void)
 }
 
 /**
- * parse_prefix - Parse level and control flags.
+ * printk_parse_prefix - Parse level and control flags.
  *
  * @text:     The terminated text message.
  * @level:    A pointer to the current level value, will be updated.

@@ -1973,7 +1973,7 @@ static inline u32 printk_caller_id(void)
  *
  * Return: The length of the parsed level and control flags.
  */
-static u16 parse_prefix(char *text, int *level,
+u16 printk_parse_prefix(const char *text, int *level,
 			enum printk_info_flags *flags)
 {
 	u16 prefix_len = 0;

@@ -2019,7 +2019,7 @@ static u16 printk_sprint(char *text, u16 size, int facility,
 	if (facility == 0) {
 		u16 prefix_len;
 
-		prefix_len = parse_prefix(text, NULL, NULL);
+		prefix_len = printk_parse_prefix(text, NULL, NULL);
 		if (prefix_len) {
 			text_len -= prefix_len;
 			memmove(text, text + prefix_len, text_len);

@@ -2068,7 +2068,7 @@ int vprintk_store(int facility, int level,
 
 	/* Extract log level or control flags. */
 	if (facility == 0)
-		parse_prefix(&prefix_buf[0], &level, &flags);
+		printk_parse_prefix(&prefix_buf[0], &level, &flags);
 
 	if (level == LOGLEVEL_DEFAULT)
 		level = default_message_loglevel;

2.31.1

Best Regards,
Petr

We have a number of systems industry-wide that have a subset of their functionality that works as follows:

1. Receive a message from local kmsg, serial console, or netconsole;
2. Apply a set of rules to classify the message;
3. Do something based on this classification (like scheduling a remediation for the machine), rinse, and repeat.

As a couple of examples of places we have this implemented just inside Facebook, although this isn't a Facebook-specific problem, we have this inside our netconsole processing (for alarm classification), and as part of our machine health checking. We use these messages to determine fairly important metrics around production health, and it's important that we get them right.

While for some kinds of issues we have counters, tracepoints, or metrics with a stable interface which can reliably indicate the issue, in order to react to production issues quickly we need to work with the interface which most kernel developers naturally use when developing: printk.

Most production issues come from unexpected phenomena, and as such usually the code in question doesn't have easily usable tracepoints or other counters available for the specific problem being mitigated. We have a number of lines of monitoring defence against problems in production (host metrics, process metrics, service metrics, etc), and where it's not feasible to reliably monitor at another level, this kind of pragmatic netconsole monitoring is essential.

As one would expect, monitoring using printk is rather brittle for a number of reasons -- most notably that the message might disappear entirely in a new version of the kernel, or that the message may change in some way that the regex or other classification methods start to silently fail.

One factor that makes this even harder is that, under normal operation, many of these messages are never expected to be hit. For example, there may be a rare hardware bug which one wants to detect if it was to ever happen again, but its recurrence is not likely or anticipated. This precludes using something like checking whether the printk in question was printed somewhere fleetwide recently to determine whether the message in question is still present or not, since we don't anticipate that it should be printed anywhere, but still need to monitor for its future presence in the long-term.

This class of issue has happened on a number of occasions, causing unhealthy machines with hardware issues to remain in production for longer than ideal. As a recent example, some monitoring around blk_update_request fell out of date and caused semi-broken machines to remain in production for longer than would be desirable.

Searching through the codebase to find the message is also extremely fragile, because many of the messages are further constructed beyond their callsite (eg. btrfs_printk and other module-specific wrappers, each with their own functionality). Even if they aren't, guessing the format and formulation of the underlying message based on the aesthetics of the message emitted is not a recipe for success at scale, and our previous issues with fleetwide machine health checking demonstrate as much.

This provides a solution to the issue of silently changed or deleted
printks: we record pointers to all printk format strings known at compile time into a new .printk_index section, both in vmlinux and modules. At runtime, this can then be iterated by looking at <debugfs>/printk/index/<module>, which emits the following format, both readable by humans and able to be parsed by machines:

    $ head -1 vmlinux; shuf -n 5 vmlinux
    # <level[,flags]> filename:line function "format"
    <5> block/blk-settings.c:661 disk_stack_limits "%s: Warning: Device %s is misaligned\n"
    <4> kernel/trace/trace.c:8296 trace_create_file "Could not create tracefs '%s' entry\n"
    <6> arch/x86/kernel/hpet.c:144 _hpet_print_config "hpet: %s(%d):\n"
    <6> init/do_mounts.c:605 prepare_namespace "Waiting for root device %s...\n"
    <6> drivers/acpi/osl.c:1410 acpi_no_auto_serialize_setup "ACPI: auto-serialization disabled\n"

This mitigates the majority of cases where we have a highly-specific printk which we want to match on, as we can now enumerate and check whether the format changed or the printk callsite disappeared entirely in userspace. This allows us to catch changes to printks we monitor earlier and decide what to do about it before it becomes problematic.

There is no additional runtime cost for printk callers or printk itself, and the assembly generated is exactly the same.

---
 MAINTAINERS                          |   5 +
 arch/arm/kernel/entry-v7m.S          |   2 +-
 arch/arm/lib/backtrace-clang.S       |   2 +-
 arch/arm/lib/backtrace.S             |   2 +-
 arch/arm/mach-rpc/io-acorn.S         |   2 +-
 arch/arm/vfp/vfphw.S                 |   6 +-
 arch/ia64/include/uapi/asm/cmpxchg.h |   4 +-
 arch/openrisc/kernel/entry.S         |   6 +-
 arch/powerpc/kernel/head_fsl_booke.S |   2 +-
 arch/um/include/shared/user.h        |   3 +-
 arch/x86/kernel/head_32.S            |   2 +-
 include/asm-generic/vmlinux.lds.h    |  13 ++
 include/linux/module.h               |   5 +
 include/linux/printk.h               |  95 ++++++++++++-
 init/Kconfig                         |  14 ++
 kernel/module.c                      |   5 +
 kernel/printk/Makefile               |   1 +
 kernel/printk/index.c                | 195 +++++++++++++++++++++++++++
 kernel/printk/printk.c               |  13 +-
 19 files changed, 353 insertions(+), 24 deletions(-)
 create mode 100644 kernel/printk/index.c

index 0bcb7f4c9d74..557bf2588f5d 100644
@@ -14878,6 +14878,11 @@ S:	Maintained
 F:	include/linux/printk.h
 F:	kernel/printk/
 
+PRINTK INDEXING
+R:	Chris Down <[email protected]>
+S:	Maintained
+F:	kernel/printk/index.c
+
 PRISM54 WIRELESS DRIVER
 M:	Luis Chamberlain <[email protected]>
 L:	[email protected]

index d0e898608d30..7bde93c10962 100644
@@ -23,7 +23,7 @@ __invalid_entry:
 	adr	r0, strerr
 	mrs	r1, ipsr
 	mov	r2, lr
-	bl	printk
+	bl	_printk
 #endif
 	mov	r0, sp
 	bl	show_regs

index 6174c45f53a5..5b2cdb1003e3 100644
@@ -202,7 +202,7 @@ finished_setup:
 1006:		adr	r0, .Lbad
 		mov	r1, loglvl
 		mov	r2, frame
-		bl	printk
+		bl	_printk
 no_frame:	ldmfd	sp!, {r4 - r9, fp, pc}
 ENDPROC(c_backtrace)
 		.pushsection __ex_table,"a"

index 872f658638d9..e8408f22d4dc 100644
@@ -103,7 +103,7 @@ for_each_frame:	tst	frame, mask		@ Check for address exceptions
 1006:		adr	r0, .Lbad
 		mov	r1, loglvl
 		mov	r2, frame
-		bl	printk
+		bl	_printk
 no_frame:	ldmfd	sp!, {r4 - r9, pc}
 ENDPROC(c_backtrace)
 		

index b9082a2a2a01..aa9bf0d771c0 100644
@@ -25,4 +25,4 @@ ENTRY(insl)
 ENTRY(outsl)
 		adr	r0, .Liosl_warning
 		mov	r1, lr
-		b	printk
+		b	_printk

index d5837bf05a9a..6f7926c9c179 100644
@@ -23,7 +23,7 @@
 #ifdef DEBUG
 	stmfd	sp!, {r0-r3, ip, lr}
 	ldr	r0, =1f
-	bl	printk
+	bl	_printk
 	ldmfd	sp!, {r0-r3, ip, lr}
 
 	.pushsection .rodata, "a"

@@ -38,7 +38,7 @@
 	stmfd	sp!, {r0-r3, ip, lr}
 	mov	r1, \arg
 	ldr	r0, =1f
-	bl	printk
+	bl	_printk
 	ldmfd	sp!, {r0-r3, ip, lr}
 
 	.pushsection .rodata, "a"

@@ -55,7 +55,7 @@
 	mov	r2, \arg2
 	mov	r1, \arg1
 	ldr	r0, =1f
-	bl	printk
+	bl	_printk
 	ldmfd	sp!, {r0-r3, ip, lr}
 
 	.pushsection .rodata, "a"

index 926c6cb1e029..2c2f3cfeaa77 100644
@@ -143,9 +143,9 @@ extern long ia64_cmpxchg_called_with_bad_pointer(void);
 do {									\
 	if (_cmpxchg_bugcheck_count-- <= 0) {				\
 		void *ip;						\
-		extern int printk(const char *fmt, ...);		\
+		extern int _printk(const char *fmt, ...);		\
 		ip = (void *) ia64_getreg(_IA64_REG_IP);		\
-		printk("CMPXCHG_BUGCHECK: stuck at %p on word %p\n", ip, (v));\
+		_printk("CMPXCHG_BUGCHECK: stuck at %p on word %p\n", ip, (v));\
 		break;							\
 	}								\
 } while (0)

index bc657e55c15f..947613f61d4a 100644
@@ -551,7 +551,7 @@ EXCEPTION_ENTRY(_external_irq_handler)
 	l.movhi r3,hi(42f)
 	l.ori	r3,r3,lo(42f)
 	l.sw    0x0(r1),r3
-	l.jal   printk
+	l.jal   _printk
 	l.sw    0x4(r1),r4
 	l.addi  r1,r1,0x8
 

@@ -681,8 +681,8 @@ _syscall_debug:
 	l.sw    -4(r1),r27
 	l.sw    -8(r1),r11
 	l.addi  r1,r1,-8
-	l.movhi r27,hi(printk)
-	l.ori   r27,r27,lo(printk)
+	l.movhi r27,hi(_printk)
+	l.ori   r27,r27,lo(_printk)
 	l.jalr  r27
 	 l.nop
 	l.addi  r1,r1,8

index a1a5c3f10dc4..08d7153ed04b 100644
@@ -858,7 +858,7 @@ KernelSPE:
 	ori	r3,r3,87f@l
 	mr	r4,r2		/* current */
 	lwz	r5,_NIP(r1)
-	bl	printk
+	bl	_printk
 #endif
 	b	interrupt_return
 #ifdef CONFIG_PRINTK

index e793e4212f0a..dd4badffdeb3 100644
@@ -38,7 +38,8 @@ extern void panic(const char *fmt, ...)
 #define UM_KERN_CONT	KERN_CONT
 
 #ifdef UML_CONFIG_PRINTK
-extern int printk(const char *fmt, ...)
+#define printk(...) _printk(__VA_ARGS__)
+extern int _printk(const char *fmt, ...)
 	__attribute__ ((format (printf, 1, 2)));
 #else
 static inline int printk(const char *fmt, ...)

index 67f590425d90..d8c64dab0efe 100644
@@ -432,7 +432,7 @@ SYM_FUNC_START(early_ignore_irq)
 	pushl 32(%esp)
 	pushl 40(%esp)
 	pushl $int_msg
-	call printk
+	call _printk
 
 	call dump_stack
 

index 17325416e2de..ddb2ff158321 100644
@@ -483,6 +483,8 @@
 									\
 	TRACEDATA							\
 									\
+	PRINTK_INDEX							\
+									\
 	/* Kernel symbol table: Normal symbols */			\
 	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
 		__start___ksymtab = .;					\

@@ -893,6 +895,17 @@
 #define TRACEDATA
 #endif
 
+#ifdef CONFIG_PRINTK_INDEX
+#define PRINTK_INDEX							\
+	.printk_index : AT(ADDR(.printk_index) - LOAD_OFFSET) {		\
+		__start_printk_index = .;				\
+		*(.printk_index)					\
+		__stop_printk_index = .;				\
+	}
+#else
+#define PRINTK_INDEX
+#endif
+
 #define NOTES								\
 	.notes : AT(ADDR(.notes) - LOAD_OFFSET) {			\
 		__start_notes = .;					\

index 8a298d820dbc..c9f1200b2312 100644
@@ -511,6 +511,11 @@ struct module {
 	struct klp_modinfo *klp_info;
 #endif
 
+#ifdef CONFIG_PRINTK_INDEX
+	unsigned int printk_index_size;
+	struct pi_entry **printk_index_start;
+#endif
+
 #ifdef CONFIG_MODULE_UNLOAD
 	/* What modules depend on me? */
 	struct list_head source_list;

index 173a474012d7..f98283a851e4 100644
@@ -781,6 +781,20 @@ config PRINTK_SAFE_LOG_BUF_SHIFT
 		     13 =>   8 KB for each CPU
 		     12 =>   4 KB for each CPU
 
+config PRINTK_INDEX
+	bool "Printk indexing debugfs interface"
+	depends on PRINTK && DEBUG_FS
+	help
+	  Add support for indexing of all printk formats known at compile time
+	  at <debugfs>/printk/index/<module>.
+
+	  This can be used as part of maintaining daemons which monitor
+	  /dev/kmsg, as it permits auditing the printk formats present in a
+	  kernel, allowing detection of cases where monitored printks are
+	  changed or no longer present.
+
+	  There is no additional runtime cost to printk with this enabled.
+
 #
 # Architectures with an unreliable sched_clock() should select this:
 #

index 25dbdaf1f337..3ad43711941a 100644
@@ -3351,6 +3351,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 						sizeof(unsigned long),
 						&mod->num_kprobe_blacklist);
 #endif
+#ifdef CONFIG_PRINTK_INDEX
+	mod->printk_index_start = section_objs(info, ".printk_index",
+					       sizeof(*mod->printk_index_start),
+					       &mod->printk_index_size);
+#endif
 #ifdef CONFIG_HAVE_STATIC_CALL_INLINE
 	mod->static_call_sites = section_objs(info, ".static_call_sites",
 					      sizeof(*mod->static_call_sites),

index eee3dc9b60a9..d118739874c0 100644
@@ -3,3 +3,4 @@ obj-y	= printk.o
 obj-$(CONFIG_PRINTK)	+= printk_safe.o
 obj-$(CONFIG_A11Y_BRAILLE_CONSOLE)	+= braille.o
 obj-$(CONFIG_PRINTK)	+= printk_ringbuffer.o
+obj-$(CONFIG_PRINTK_INDEX)	+= index.o

index 26d52aea9969..085fb05c957a 100644
@@ -2185,10 +2185,13 @@ int vprintk_default(const char *fmt, va_list args)
 EXPORT_SYMBOL_GPL(vprintk_default);
 
 /**
- * printk - print a kernel message
+ * _printk - print a kernel message
  * @fmt: format string
  *
- * This is printk(). It can be called from any context. We want it to work.
+ * This is _printk(). It can be called from any context. We want it to work.
+ *
+ * If printk indexing is enabled, _printk() is called from printk_index_wrap.
+ * Otherwise, printk is simply #defined to _printk.
  *
  * We try to grab the console_lock. If we succeed, it's easy - we log the
  * output and call the console drivers.  If we fail to get the semaphore, we

@@ -2205,7 +2208,7 @@ EXPORT_SYMBOL_GPL(vprintk_default);
  *
  * See the vsnprintf() documentation for format string extensions over C99.
  */
-asmlinkage __visible int printk(const char *fmt, ...)
+asmlinkage __visible int _printk(const char *fmt, ...)
 {
 	va_list args;
 	int r;

@@ -2216,7 +2219,7 @@ asmlinkage __visible int printk(const char *fmt, ...)
 
 	return r;
 }
-EXPORT_SYMBOL(printk);
+EXPORT_SYMBOL(_printk);
 
 #else /* CONFIG_PRINTK */
 

@@ -3201,7 +3204,7 @@ int vprintk_deferred(const char *fmt, va_list args)
 	return r;
 }
 
-int printk_deferred(const char *fmt, ...)
+int _printk_deferred(const char *fmt, ...)
 {
 	va_list args;
 	int r;

2.31.1

Not sure that has what Andrew keeps in his quilt.

Chris, to avoid such issues, use git format-patch --base ... when sending a series / patches.

With Best Regards,
Andy Shevchenko

Yup, it compiles fine on linux-next where the funtion is declared via a patch from Andrew's tree.

Best Regards,
Petr

Ah, I forgot about that. Will do, thanks.

This version looks fine to me:

Best Regards,
Petr

This issue already exists before this patch and has done for a very long time, just the error message changed. Let's fix it in a different series.

It is build when CONFIG_PRINTK is not set. Do I get it correctly? Yes, I agree that it is an older problem and we could fix it later.

Best Regards,
Petr

While for most kinds of issues we have counters, tracepoints, or metrics with a stable interface which can reliably be used to indicate issues, in order to react to production issues quickly we sometimes need to work with the interface which most kernel developers naturally use when
developing: printk, and printk-esques like dev_printk.

dev_printk is by far the most likely custom subsystem printk to benefit from the printk indexing infrastructure, since niche device issues brought about by production changes, firmware upgrades, and the like are one of the most common things that we need printk infrastructure's assistance to monitor.

Often these errors were never expected to practically manifest in reality, and exhibit in code without extensive (or any) metrics present. As such, there are typically very few options for issue detection available to those with large fleets at the time the incident happens, and we thus benefit strongly from monitoring netconsole in these instances.

As such, add the infrastructure for dev_printk to be indexed in the printk index. Even on a minimal kernel config, the coverage of the base kernel's printk index is significantly improved:

Before:

[root@ktst ~]# wc -l /sys/kernel/debug/printk/index/vmlinux 4497 /sys/kernel/debug/printk/index/vmlinux

After:

[root@ktst ~]# wc -l /sys/kernel/debug/printk/index/vmlinux 5573 /sys/kernel/debug/printk/index/vmlinux

In terms of implementation, in order to trivially disambiguate them, dev_printk is now a macro which wraps _dev_printk.

---
 drivers/base/core.c        |  6 ++--
 include/linux/dev_printk.h | 66 ++++++++++++++++++++++++++++----------
 2 files changed, 52 insertions(+), 20 deletions(-)

index b8a8c96dca58..31a24b4e27cd 100644
@@ -4579,8 +4579,8 @@ static void __dev_printk(const char *level, const struct device *dev,
 		printk("%s(NULL device *): %pV", level, vaf);
 }
 
-void dev_printk(const char *level, const struct device *dev,
-		const char *fmt, ...)
+void _dev_printk(const char *level, const struct device *dev,
+		 const char *fmt, ...)
 {
 	struct va_format vaf;
 	va_list args;

@@ -4594,7 +4594,7 @@ void dev_printk(const char *level, const struct device *dev,
 
 	va_end(args);
 }
-EXPORT_SYMBOL(dev_printk);
+EXPORT_SYMBOL(_dev_printk);
 
 #define define_dev_printk_level(func, kern_level)		\
 void func(const struct device *dev, const char *fmt, ...)	\

2.31.1

Best Regards,
Petr

The patchset looks ready for linux-next from my POV. I could fixup the messages as suggested by Andy when pushing.

Well, I would still like to get acks from:

+ Andy for the 1st patch
+ Jessica for the changes in the module loader code in 4th patch.

Best Regards,
Petr

Assuming that Chris indeed spent time on checking string_esape_mem() users along with %*pE (and all its variations with hardcoded length) and haven't found any problems,

With Best Regards,
Andy Shevchenko

Thanks! Probably worth documenting my methodology :-)

Forgive the lack of wrapping -- I suspect it will probably make this easier to read.

% git grep '"[^"]*%[*0-9]*pE[^"]*"'
drivers/gpu/drm/drm_dp_cec.c: seq_printf(file, "ID: %*pE\n",
drivers/gpu/drm/drm_dp_dual_mode_helper.c: drm_dbg_kms(dev, "DP dual mode HDMI ID: %*pE (err %zd)\n",
drivers/gpu/drm/drm_dp_helper.c: "%s: DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
drivers/net/wireless/intel/ipw2x00/ipw2100.c: IPW_DEBUG_INFO("%s: Associated with '%*pE' at %s, channel %d (BSSID=%pM)\n",
drivers/net/wireless/intel/ipw2x00/ipw2100.c: IPW_DEBUG_HC("SSID: '%*pE'\n", ssid_len, essid);
drivers/net/wireless/intel/ipw2x00/ipw2100.c: "disassociated: '%*pE' %pM\n", priv->essid_len, priv->essid,
drivers/net/wireless/intel/ipw2x00/ipw2100.c: IPW_DEBUG_WX("Setting ESSID: '%*pE' (%d)\n", length, essid, length);
drivers/net/wireless/intel/ipw2x00/ipw2100.c: IPW_DEBUG_WX("Getting essid: '%*pE'\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "associated: '%*pE' %pM\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "deauthenticated: '%*pE' %pM: (0x%04X) - %s\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "authenticated: '%*pE' %pM\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "disassociated: '%*pE' %pM\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "authenticated: '%*pE' %pM\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: "deauthenticated: '%*pE' %pM\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded due to capability mismatch.\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of non-network ESSID.\n",
drivers/net/wireless/intel/ipw2x00/ipw2200.c: IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of ESSID mismatch: '%*pE'.\n",

  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE excluded because newer than current network.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE excluded because newer than current network.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of age: %ums.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of channel mismatch: %d != %d.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of privacy mismatch: %s != %s.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of the same BSSID match: %pM.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of invalid frequency/mode combination.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because configured rate mask excludes AP mandatory rate.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_MERGE("Network '%*pE (%pM)' excluded because of no compatible rates.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:   IPW_DEBUG_MERGE("Network '%*pE (%pM)' is a viable match.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:                   IPW_DEBUG_MERGE("remove network %*pE\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded due to capability mismatch.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:                   IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of non-network ESSID.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:                   IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of ESSID mismatch: '%*pE'.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because '%*pE (%pM)' has a stronger signal.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of storming (%ums since last assoc attempt).\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of age: %ums.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of channel mismatch: %d != %d.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of privacy mismatch: %s != %s.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of BSSID mismatch: %pM.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of invalid frequency/mode combination.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of invalid channel in current GEO\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because configured rate mask excludes AP mandatory rate.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_ASSOC("Network '%*pE (%pM)' excluded because of no compatible rates.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:   IPW_DEBUG_ASSOC("Network '%*pE (%pM)' is a viable match.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_INFO("ESSID locked to '%*pE'\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:   IPW_DEBUG_ASSOC("%ssociation attempt: '%*pE', channel %d, 802.11%c [%d], %s[:%s], enc=%s%s%s%c%c\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:   IPW_DEBUG(IPW_DL_STATE, "associating: '%*pE' %pM\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:                   IPW_DEBUG_ASSOC("Expired '%*pE' (%pM) from network list.\n",
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:   IPW_DEBUG_WX("Setting ESSID: '%*pE' (%d)\n", length, extra, length);
  drivers/net/wireless/intel/ipw2x00/ipw2200.c:           IPW_DEBUG_WX("Getting essid: '%*pE'\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:                 LIBIPW_DEBUG_MGMT("WLAN_EID_SSID: '%*pE' len=%d.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:         LIBIPW_DEBUG_SCAN("Filtered out '%*pE (%pM)' network.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c: LIBIPW_DEBUG_SCAN("'%*pE' (%pM): %c%c%c%c %c%c%c%c-%c%c%c%c %c%c%c%c\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:         LIBIPW_DEBUG_SCAN("Dropped '%*pE' (%pM) via %s.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:                 LIBIPW_DEBUG_SCAN("Expired '%*pE' (%pM) from network list.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:         LIBIPW_DEBUG_SCAN("Adding '%*pE' (%pM) via %s.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_rx.c:         LIBIPW_DEBUG_SCAN("Updating '%*pE' (%pM) via %s.\n",
  drivers/net/wireless/intel/ipw2x00/libipw_wx.c:                 LIBIPW_DEBUG_SCAN("Not showing network '%*pE (%pM)' due to age (%ums).\n",
  drivers/net/wireless/intel/ipw2x00/libipw_wx.c:         LIBIPW_DEBUG_WX("Setting key %d to '%*pE' (%d:%d bytes)\n",
  drivers/net/wireless/intersil/hostap/hostap_proc.c:     seq_printf(m, "%*pE", (int)bss->ssid_len, bss->ssid);
  drivers/net/wireless/marvell/libertas/cfg.c:                    lbs_deb_scan("scan: %pM, capa %04x, chan %2d, %*pE, %d dBm\n",
  drivers/net/wireless/marvell/libertas/mesh.c:   lbs_deb_cmd("mesh config action %d type %x channel %d SSID %*pE\n",
  drivers/platform/olpc/olpc-xo175-ec.c:  dev_dbg(&priv->spi->dev, "got debug string [%*pE]\n",
  drivers/platform/surface/surface3_power.c:      snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
  drivers/platform/surface/surface3_power.c:      snprintf(bix->OEM, ARRAY_SIZE(bix->OEM), "%3pE", buf);
  drivers/platform/x86/wmi.c:             pr_info("\tobject_id: %2pE\n", g->object_id);
  drivers/scsi/scsi_lib.c:                        id_size = snprintf(id, id_len, "t10.%*pE",
  drivers/soc/qcom/cmd-db.c:                      seq_printf(seq, "0x%05x: %*pEp", le32_to_cpu(ent->addr),
  drivers/staging/rtl8192e/rtllib.h:      snprintf(escaped, sizeof(escaped), "%*pE", essid_len, essid);
  drivers/staging/rtl8192u/ieee80211/ieee80211.h: snprintf(escaped, sizeof(escaped), "%*pE", essid_len, essid);
  drivers/staging/wlan-ng/prism2sta.c:            netdev_info(wlandev->netdev, "Prism2 card SN: %*pE\n",
  drivers/thunderbolt/xdomain.c:  return sprintf(buf, "%*pE\n", (int)strlen(svc->key), svc->key);
  drivers/tty/mips_ejtag_fdc.c:           dev_dbg(priv->dev, "%s%u: out %08x: \"%*pE%*pE\"\n",
  drivers/tty/mips_ejtag_fdc.c:           dev_dbg(priv->dev, "%s%u: in  %08x: \"%*pE\"\n",
  drivers/tty/serial/serial_core.c:       pr_info("SysRq is enabled by magic sequence '%*pE' on serial\n",
  fs/overlayfs/overlayfs.h:       pr_debug("getxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
  fs/overlayfs/overlayfs.h:       pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
  lib/test_printf.c:      test("(null)", "%pE", NULL);
  lib/test_printf.c:      test("(efault)", "%pE", ERR_PTR(-11));
  lib/test_printf.c:      test("(efault)", "%pE", PTR_INVALID);

net/ceph/debugfs.c: seq_printf(s, "%*pE/%*pE\t0x%x",
net/ceph/debugfs.c: seq_printf(s, "%*pE\t0x%x", t->target_oid.name_len,

For all ESSID cases, there's no clear standard, but according to Cisco, double quote is illegal in SSID (although there's no formalised standard). At the very least it's extremely unusual, and even if it happens, I don't see how it could cause problems for the cases here.

- drivers/gpu/drm/drm_dp_cec.c: Device ID, chance of quotes is approaching zero.
- drivers/gpu/drm/drm_dp_dual_mode_helper.c: Debugging message only.
- drivers/gpu/drm/drm_dp_helper.c: Debugging message only.
- drivers/net/wireless/intel/ipw2x00/ipw2100.c: ESSID case, debugging message only.
- drivers/net/wireless/intel/ipw2x00/ipw2200.c: ESSID case, debugging message only.
- drivers/net/wireless/intersil/hostap/hostap_proc.c: ESSID case, /proc.
- drivers/net/wireless/marvell/libertas/cfg.c: ESSID case, debugging message only.
- drivers/net/wireless/marvell/libertas/mesh.c: ESSID case, debugging message only.
- drivers/platform/olpc/olpc-xo175-ec.c: Debugging message only.
- drivers/platform/surface/surface3_power.c: It wouldn't make sense for either the OEM or serial to contain quotes, especially considering how low level this is.
- drivers/platform/x86/wmi.c: Debug dump only, there's not gonna be quotes in 2 character escaping anyway.
- drivers/scsi/scsi_lib.c: Reading vendor ID. Passed back as length anyway, so that's fine -- it just needs to be unique.
- drivers/soc/qcom/cmd-db.c: Debug dump, and judging by the code no way it's gonna contain quotes.
- drivers/staging/rtl8192e/rtllib.h: ESSID case, only used for netdev_dbg messages anyway.
- drivers/staging/rtl8192u/ieee80211/ieee80211.h: Ditto rtllib.h.
- drivers/staging/wlan-ng/prism2sta.c: Serial number, it's not gonna contain quotes. For debugging only anyway.
- drivers/thunderbolt/xdomain.c: Used in key_show, which is used as a rare device_attribute. Only used for device debug.
- drivers/tty/mips_ejtag_fdc.c: Debugging messages only.
- drivers/tty/serial/serial_core.c: Debugging message only, and benefits from quoting (but seems highly unlikely it would be there anyway).
- fs/overlayfs/overlayfs.h: Debugging messages only, and would actually even benefit from the new quoting.
- lib/test_printf.c: None of these have quotes.
- net/ceph/debugfs.c: Debugging only, and looks unlikely to be affected regardless.

(And forgot to mention I did the same for ESCAPE_SPECIAL explicit users and those using masks including it, feel free to let me know if you want those as well.)

+++ Petr Mladek [17/06/21 12:10 +0200]:

Hi Petr, the module.{c,h} changes are fine by me:

Thank you,

Jessica

They provided the Acks, so that we could push it.

Andrew, this patchset depends on seq_file and string_helpers changes that are in -mm tree:

lib-string_helpers-switch-to-use-bit-macro.patch lib-string_helpers-move-escape_np-check-inside-else-branch-in-a-loop.patch lib-string_helpers-drop-indentation-level-in-string_escape_mem.patch lib-string_helpers-introduce-escape_na-for-escaping-non-ascii.patch lib-string_helpers-introduce-escape_nap-to-escape-non-ascii-and-non-printable.patch lib-string_helpers-allow-to-append-additional-characters-to-be-escaped.patch lib-test-string_helpers-print-flags-in-hexadecimal-format.patch lib-test-string_helpers-get-rid-of-trailing-comma-in-terminators.patch lib-test-string_helpers-add-test-cases-for-new-features.patch maintainers-add-myself-as-designated-reviewer-for-generic-string-library.patch seq_file-introduce-seq_escape_mem.patch seq_file-add-seq_escape_str-as-replica-of-string_escape_str.patch seq_file-convert-seq_escape-to-use-seq_escape_str.patch nfsd-avoid-non-flexible-api-in-seq_quote_mem.patch seq_file-drop-unused-_escape_mem_ascii.patch

Would you mind to take this patchset via -mm tree as well, please?

You were not in CC. Should Chris send v8 with all the Acks and you in CC?

Best Regards,
Petr

We're at -rc7, so I wouldn't be inclined to merge significant feature work at this time. I suggest a resend after -rc1, at which time the above changes will be in mainline.

Fair enough. Let's postpone this patchset for 5.15. I am going to push it into the printk tree after the 5.14 merge window.

Best Regards,
Petr

The patchset has been committed into printk/linux.git, branch for-4.14-printk-index.

I am sorry for the delay. I was partly off and partly busy.

Best Regards,
Petr

It is actually for-5.15-printk-index

I am sorry for the confusion.

Best Regards,
Petr