From: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
Subject: [PATCH 3/3] makedumpfile: use cycle detection when parsing the prink log_buf
Date: Mon, 7 Mar 2022 18:23:22 +0100 [thread overview]
Message-ID: <20220307172322.7909-4-prudo@redhat.com> (raw)
In-Reply-To: <20220307172322.7909-1-prudo@redhat.com>
The old printk mechanism (> v3.5.0 and < v5.10.0) had a fixed size
buffer (log_buf) that contains all messages. The location for the next
message is stored in log_next_idx. In case the log_buf runs full
log_next_idx wraps around and starts overwriting old messages@the
beginning of the buffer. The wraparound is denoted by a message with
msg->len == 0.
Following the behavior described above blindly in makedumpfile is
dangerous as e.g. a memory corruption could overwrite (parts of) the
log_buf. If the corruption adds a message with msg->len == 0 this leads
to an endless loop when dumping the dmesg with makedumpfile appending
the messages up to the corruption over and over again to the output file
until file system is full. Fix this by using cycle detection and aboard
once one is detected.
While at it also verify that the index is within the log_buf and thus
guard against corruptions with msg->len != 0.
Fixes: 36c2458 ("[PATCH] --dump-dmesg fix for post 3.5 kernels.")
Reported-by: Audra Mitchell <aubaker@redhat.com>
Suggested-by: Dave Wysochanski <dwysocha@redhat.com>
Signed-off-by: Philipp Rudo <prudo@redhat.com>
---
makedumpfile.c | 42 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index edf128b..2738d16 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -15,6 +15,7 @@
*/
#include "makedumpfile.h"
#include "print_info.h"
+#include "detect_cycle.h"
#include "dwarf_info.h"
#include "elf_info.h"
#include "erase_info.h"
@@ -5528,10 +5529,11 @@ dump_dmesg()
unsigned long index, log_buf, log_end;
unsigned int log_first_idx, log_next_idx;
unsigned long long first_idx_sym;
+ struct detect_cycle *dc = NULL;
unsigned long log_end_2_6_24;
unsigned log_end_2_6_25;
char *log_buffer = NULL, *log_ptr = NULL;
- char *idx;
+ char *idx, *next_idx;
/*
* log_end has been changed to "unsigned" since linux-2.6.25.
@@ -5679,12 +5681,47 @@ dump_dmesg()
goto out;
}
idx = log_buffer + log_first_idx;
+ dc = dc_init(idx, log_buffer, log_next);
while (idx != log_buffer + log_next_idx) {
log_ptr = log_from_idx(idx, log_buffer);
if (!dump_log_entry(log_ptr, info->fd_dumpfile,
info->name_dumpfile))
goto out;
- idx = log_next(idx, log_buffer);
+ if (dc_next(dc, (void **) &next_idx)) {
+ unsigned long len;
+ char *first;
+
+ /* Clear everything we have already written... */
+ ftruncate(info->fd_dumpfile, 0);
+ lseek(info->fd_dumpfile, 0, SEEK_SET);
+
+ /* ...and only write up to the corruption. */
+ dc_find_start(dc, (void **) &first, &len);
+ idx = log_buffer + log_first_idx;
+ while (len) {
+ log_ptr = log_from_idx(idx, log_buffer);
+ if (!dump_log_entry(log_ptr,
+ info->fd_dumpfile,
+ info->name_dumpfile))
+ goto out;
+ idx = log_next(idx, log_buffer);
+ len--;
+ }
+ ERRMSG("Cycle when parsing dmesg detected.\n");
+ ERRMSG("The printk log_buf is most likely corrupted.\n");
+ ERRMSG("log_buf = 0x%lx, idx = 0x%lx\n", log_buf, idx - log_buffer);
+ close_files_for_creating_dumpfile();
+ goto out;
+ }
+ if (next_idx < log_buffer ||
+ next_idx > log_buffer + log_buf_len - SIZE(printk_log)) {
+ ERRMSG("Index outside log_buf detected.\n");
+ ERRMSG("The printk log_buf is most likely corrupted.\n");
+ ERRMSG("log_buf = 0x%lx, idx = 0x%lx\n", log_buf, idx - log_buffer);
+ close_files_for_creating_dumpfile();
+ goto out;
+ }
+ idx = next_idx;
}
if (!close_files_for_creating_dumpfile())
goto out;
@@ -5694,6 +5731,7 @@ dump_dmesg()
out:
if (log_buffer)
free(log_buffer);
+ free(dc);
return ret;
}
--
2.35.1
next prev parent reply other threads:[~2022-03-07 17:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-07 17:23 [PATCH 0/3] makedumpfile: harden parsing of old prink buffer Philipp Rudo
2022-03-07 17:23 ` [PATCH 1/3] makedumpfile: add generic cycle detection Philipp Rudo
2022-03-07 17:23 ` [PATCH 2/3] makedumpfile: use pointer arithmetics for dump_dmesg Philipp Rudo
2022-03-09 8:25 ` David Wysochanski
2022-03-07 17:23 ` Philipp Rudo [this message]
2022-03-09 8:48 ` [PATCH 3/3] makedumpfile: use cycle detection when parsing the prink log_buf David Wysochanski
2022-03-10 14:33 ` Philipp Rudo
2022-03-11 7:59 ` HAGIO KAZUHITO =?unknown-8bit?b?6JCp5bC+IOS4gOS7gQ==?=
2022-03-11 12:28 ` Philipp Rudo
2022-03-08 17:16 ` [PATCH 0/3] makedumpfile: harden parsing of old prink buffer David Wysochanski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220307172322.7909-4-prudo@redhat.com \
--to=prudo@redhat.com \
--cc=kexec@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox