Hi, David
Thank you for the update.
On Wed, Sep 13, 2023 at 8:00 PM <crash-utility-request(a)redhat.com> wrote:
Date: Tue, 12 Sep 2023 18:11:33 -0600
From: David Mair <dmair(a)suse.com>
To: crash-utility(a)redhat.com
Subject: [Crash-utility] [PATCH] In verify_version() don't require
specific syment type values for linux_banner symbol to get it's
address
Message-ID: <55b4eb1f-8cdc-eca4-e370-1eed12133681(a)suse.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
verify_version() in kernel.c gets a struct syment for linux_banner using
symbol_search() and uses the value member of the result as the address of
linux_banner in some cases based on the type member's value in the same
struct syment. A small number of coredumps with an unhandled type ('B' or
'b') for linux_banner result in the address of linux_banner being loaded
from the actual linux_banner data. This fails because the first ulong of
the linux_banner ASCII text is treated as a dumped kernel address and
attempting to access that in the core fails.
Based on a suggestion from Kazu, continue to get the struct syment for
linux_banner using symbol_search(). Also use get_symbol_type() for
linux_banner and use the result of that to decide where to get the
linux_banner address from, disregarding the syment type member. If
get_symbol_type() reports a TYPE_CODE_ARRAY (and by default with a warning)
use the syment value member as the linux_banner address. If
get_symbol_type() reports a TYPE_CODE_PTR read the address of linux_banner
using get_symbol_data().
The else block doesn't strictly require braced content for a single switch
statement but braces are included to match style of locally similar cases.
Signed-off-by: David Mair <dmair(a)suse.com>
---
diff --git a/kernel.c b/kernel.c
index 988206b..6dcf414 100644
--- a/kernel.c
+++ b/kernel.c
@@ -1076,13 +1076,21 @@ verify_version(void)
if (!(sp = symbol_search("linux_banner")))
error(FATAL, "linux_banner symbol does not exist?\n");
- else if ((sp->type == 'R') || (sp->type == 'r') ||
- (THIS_KERNEL_VERSION >= LINUX(2,6,11) && (sp->type ==
'D'
|| sp->type == 'd')) ||
- (machine_type("ARM") && sp->type == 'T')
||
- (machine_type("ARM64")))
- linux_banner = symbol_value("linux_banner");
- else
- get_symbol_data("linux_banner", sizeof(ulong),
&linux_banner);
+ else {
+ switch (get_symbol_type("linux_banner", NULL, NULL))
+ {
+ case TYPE_CODE_ARRAY:
+ linux_banner = sp->value;
+ break;
+ case TYPE_CODE_PTR:
+ get_symbol_data("linux_banner", sizeof(ulong),
&linux_banner);
+ break;
+ default:
+ error(WARNING, "linux_banner is unknown type\n");
+ linux_banner = sp->value;
+ break;
+ }
+ }
This change looks good to me. So: Ack
Thanks.
Lianbo
if (!IS_KVADDR(linux_banner))
error(WARNING, "invalid linux_banner pointer: %lx\n",