All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] kernel/printk: RT-adaption of kmsg_dump breaks mtdoops
@ 2021-09-17 10:24 Walter Stoll
  2021-09-17 11:36 ` John Ogness
  0 siblings, 1 reply; 3+ messages in thread
From: Walter Stoll @ 2021-09-17 10:24 UTC (permalink / raw)
  To: linux-rt-users

Effect observed
---------------

We want to store kernel oops logs on a NAND flash partition by means of the
MTD Oops/Panic console logger/driver. However, no log is generated.

We observed the effect with kernel version v5.4.138-rt62. However, we think
that the most recent kernel exhibits the same behavior because the structure of
the sources in question (see below) did not change.


Root cause
----------

Further investigations revealed, than the issue is due to the realtime adaption
of the kmsg_dump() function.

Non realtime version of kmsg_dump():
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/printk/printk.c?h=linux-5.4.y#n3149

Realtime version of kmsg_dump():
https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git/tree/kernel/printk/printk.c?h=v5.4-rt#n2904

In our case, the the mtd_panic_write() function returns with error when the
kernel panics. As a result, no log is created. Call chain:
... panic() -> kmsg_dump() -> mtdoops_do_dump() -> mtdoops_write()
    mtd_panic_write()

Note that within the kmsg_dump() function, a locally create kmsg_dumper object
is passed to the dumper_local.dump(&dumper_local, reason) call. However the
callee mtdoops_do_dump() function expects this object being embedded in a
mtdoops_context object, see
https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git/tree/drivers/mtd/mtdoops.c?h=v5.4-rt#n272

This is obviously not the case, therefore the error. Note that this could even
led to a security issue.


Bug fix proposal
----------------

We fixed the bug locally by applying the patch below. Note that we did not
modify the kmsg_dump() function. Instead we modified the mtdoops_do_dump()
function which seems for us the cleaner way.


diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 774970bfcf85..317ecf47fce8 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -269,8 +269,7 @@ static void find_next_position(struct mtdoops_context *cxt)
 static void mtdoops_do_dump(struct kmsg_dumper *dumper,
 			    enum kmsg_dump_reason reason)
 {
-	struct mtdoops_context *cxt = container_of(dumper,
-			struct mtdoops_context, dump);
+	struct mtdoops_context *cxt = &oops_cxt;
 
 	/* Only dump oopses if dump_oops is set */
 	if (reason == KMSG_DUMP_OOPS && !dump_oops)


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-17 12:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 10:24 [BUG] kernel/printk: RT-adaption of kmsg_dump breaks mtdoops Walter Stoll
2021-09-17 11:36 ` John Ogness
2021-09-17 12:03   ` AW: " Walter Stoll

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.