Re: [Crash-utility] [External Mail]Re: [PATCH] Fix a segfault in setup_ikconfig.

Hi Dave, 1. I'm requesting the arm64.c change as well. Because some chip makers would like to define their own section size bits in kernel config to support memory hotplug, which is not same as ARM64 default SECTION_SIZE_BITS value: 30, such as QCOM SDM8150 or 8250 borads. Although linux kernel maintain line still define default SECTION_SIZE_BITS value: 30 as section_size_bits for sparsemem. The patch has detailed descript in the email "[PATCH] Fix for the determination of the ARM64 SECTION_SIZE_BITS".

2. The output of "sys config" for my dumpfile has been uploaded, which shows the last line is empty. it occurs a segment fault when add the last empty line into ikconfig entry. I agree that the use of STRNEQ() is better than strncmp(). Best regards, Qiwu -----Original Message----- From: Dave Anderson <anderson(a)redhat.com&gt; Sent: Wednesday, September 18, 2019 3:22 AM To: 陈启武 <chenqiwu(a)xiaomi.com&gt; Cc: crash-utility(a)redhat.com Subject: [External Mail]Re: [PATCH] Fix a segfault in setup_ikconfig. ----- Original Message ----- > > Hi Anderson, > I want to introduce a patch to your crash tool project. It’s a bugfix > for a segfault in setup_ikconfig. > We add an ikconfig entry only if ent[0] != '#', it is not an advisable > condition because there is a potential segfault risk if ent is gibberish. Using your dumpfile that has the bogus "ent" string, can you show me what the output of "sys config" looks like? Dave > I explain the reproducing steps about this segfault case: > I try to apply the following patch to crash 7.2.6++ code for a test. > --- a/arm64.c > +++ b/arm64.c > @@ -32,6 +32,7 @@ static int verify_kimage_voffset(void); static void > arm64_calc_kimage_voffset(void); static void > arm64_calc_phys_offset(void); static void > arm64_calc_virtual_memory_ranges(void); > +static void arm64_get_section_size_bits(void); > static int arm64_kdump_phys_base(ulong *); static ulong > arm64_processor_speed(void); static void arm64_init_kernel_pgd(void); > @@ -375,7 +376,11 @@ arm64_init(int when) > > case POST_GDB: > arm64_calc_virtual_memory_ranges(); > - machdep->section_size_bits = _SECTION_SIZE_BITS; > + arm64_get_section_size_bits(); > + if (CRASHDEBUG(1)) { > + fprintf(fp, "SECTION_SIZE_BITS: %ld\n", > machdep->section_size_bits); > + } > + > if (!machdep->max_physmem_bits) { > if ((string = > pc->read_vmcoreinfo("NUMBER(MAX_PHYSMEM_BITS)"))) { > machdep->max_physmem_bits = > atol(string); @@ -1055,6 +1060,32 @@ arm64_calc_phys_offset(void) > fprintf(fp, "using %lx as phys_offset\n", > ms->phys_offset); } > > +/* > + * Determine SECTION_SIZE_BITS either by reading VMCOREINFO or the > +kernel > + * config, otherwise borrow the 64-bit ARM default definiton. > + */ > +static void > +arm64_get_section_size_bits(void) > +{ > + int ret; > + char *string; > + > + if ((string = pc->read_vmcoreinfo("NUMBER(SECTION_SIZE_BITS)"))) { > + machdep->section_size_bits = atol(string); > + free(string); > + return; > + } > + > + if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL)) > + == > IKCONFIG_Y) { > + if ((ret = > + get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS", > &string)) == IKCONFIG_STR) { > + machdep->section_size_bits = atol(string); > + free(string); > + return; > + } > + } else { > + machdep->section_size_bits = _SECTION_SIZE_BITS; > + } > +} > > Then I make and load the dumpfiles by crash, it occurs a segment fault > as > below: > crash[31000]: segfault at 0 ip 00007f0fb24d98d1 sp 00007fff1703f7e8 > error 4 in libc-2.26.so[7f0fb235b000+1d6000] > > So I add debug to find out the segfault reason, It occurred in > setup_ikconfig-> add_ikconfig_entry. > add_ikconfig_entry: ▒▒▒U //The last ent is a gibberish, lead > to segfault > > I think the most advisable judgement is if an ikconfig entry start > with "CONFIG_". I debug by the following patch and never reproduce > segfault again. > diff --git a/kernel.c b/kernel.c > index 7804aef..d023c87 100644 > --- a/kernel.c > +++ b/kernel.c > @@ -10144,7 +10144,7 @@ static int setup_ikconfig(char *config) > while (whitespace(*ent)) > ent++; > > - if (ent[0] != '#') { > + if (!strncmp(ent, "CONFIG_", strlen("CONFIG_"))) { > add_ikconfig_entry(ent, > &ikconfig_all[kt->ikconfig_ents++]); > if (kt->ikconfig_ents == IKCONFIG_MAX) { > > Thanks for your review. I’m looking forward to your favourable reply! > > Best regards, > Qiwu > > > > > #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部 > 或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ > This e-mail and its attachments contain confidential information from > XIAOMI, which is intended only for the person or entity whose address > is listed above. Any use of the information contained herein in any > way (including, but not limited to, total or partial disclosure, > reproduction, or dissemination) by persons other than the intended > recipient(s) is prohibited. If you receive this e-mail in error, > please notify the sender by phone or email immediately and delete > it!******/# > #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#