[PATCH v2 0/4] sadump, kaslr: fix failure of calculating kaslr_offset due to an sadump format restriction
5 a.m.
This patch series fix failure of calculating kaslr_offset due to an
sadump format restriction.
The main part is the 4-th patch. I found some small bugs during this
work and the 2nd and 3rd are to fix them and are also needed for the
4-th patch.
This issue occurs also in makedumpfile, and I have already posted the
corresponding patch for makedumpfile at:
http://lists.infradead.org/pipermail/kexec/2020-July/020852.html
ChangeLog:
v1=>v2:
- Add Signed-off-by: in each commit description
- 4/4: Remove unused zero variable in get_sadump_smram_cpu_state_any()
- 2/4: Fix mismatch of the symbol name and the member of the symbol table
HATAYAMA Daisuke (4):
symbols: Add linux_banner_vmlinux in symbol table
symbols: fix initialization of st->{pti_init,kaiser}_vmlinux
kaslr: cleanup how to set values to the arguments of
calc_kaslr_offset()
kaslr: fix failure of calculating kaslr_offset due to an sadump format
restriction
defs.h | 1 +
kaslr_helper.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------------
sadump.c | 20 +++++++++++--------
symbols.c | 25 +++++++++++++++++++-----
4 files changed, 81 insertions(+), 26 deletions(-)
--
1.8.3.1
5 a.m.
New subject: [PATCH v2 1/4] symbols: Add linux_banner_vmlinux in symbol table
From: HATAYAMA Daisuke <d.hatayama(a)jp.fujitsu.com>
Add linux_banner_vmlinux in symbol table, which we'll later use in
calc_kaslr_offset() to do a sanity check in calculation of
kaslr_offset and phys_base.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
defs.h | 1 +
symbols.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/defs.h b/defs.h
index d7adb23..17e9876 100644
--- a/defs.h
+++ b/defs.h
@@ -2714,6 +2714,7 @@ struct symbol_table_data {
ulong pti_init_vmlinux;
ulong kaiser_init_vmlinux;
int kernel_symbol_type;
+ ulong linux_banner_vmlinux;
};
/* flags for st */
diff --git a/symbols.c b/symbols.c
index 3b1f08a..b9de4a1 100644
--- a/symbols.c
+++ b/symbols.c
@@ -3226,6 +3226,11 @@ dump_symbol_table(void)
fprintf(fp, " kaiser_init_vmlinux: (unused)\n");
}
+ if (SADUMP_DUMPFILE())
+ fprintf(fp, "linux_banner_vmlinux: %lx\n", st->linux_banner_vmlinux);
+ else
+ fprintf(fp, "linux_banner_vmlinux: (unused)\n");
+
fprintf(fp, " symval_hash[%d]: %lx\n", SYMVAL_HASH,
(ulong)&st->symval_hash[0]);
@@ -12687,6 +12692,11 @@ numeric_forward(const void *P_x, const void *P_y)
else if (STREQ(y->name, "idt_table"))
st->idt_table_vmlinux = valueof(y);
+ if (STREQ(x->name, "linux_banner"))
+ st->linux_banner_vmlinux = valueof(x);
+ else if (STREQ(y->name, "linux_banner"))
+ st->linux_banner_vmlinux = valueof(y);
+
if (STREQ(x->name, "saved_command_line"))
st->saved_command_line_vmlinux = valueof(x);
else if (STREQ(y->name, "saved_command_line"))
--
1.8.3.1
5 a.m.
New subject: [PATCH v2 2/4] symbols: fix initialization of st->{pti_init, kaiser}_vmlinux
From: HATAYAMA Daisuke <d.hatayama(a)jp.fujitsu.com>
In numeric_forward(), care must be taken both for x- and y- positions,
but either of kaiser_init and pti_init is only for x- or y- position
only. Fix this. Also, move the code in an appropriate position
according to each symbol name in the alphabetical order.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
symbols.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/symbols.c b/symbols.c
index b9de4a1..2fecaee 100644
--- a/symbols.c
+++ b/symbols.c
@@ -12692,20 +12692,25 @@ numeric_forward(const void *P_x, const void *P_y)
else if (STREQ(y->name, "idt_table"))
st->idt_table_vmlinux = valueof(y);
+ if (STREQ(x->name, "kaiser_init"))
+ st->kaiser_init_vmlinux = valueof(x);
+ else if (STREQ(y->name, "kaiser_init"))
+ st->kaiser_init_vmlinux = valueof(y);
+
if (STREQ(x->name, "linux_banner"))
st->linux_banner_vmlinux = valueof(x);
else if (STREQ(y->name, "linux_banner"))
st->linux_banner_vmlinux = valueof(y);
+ if (STREQ(x->name, "pti_init"))
+ st->pti_init_vmlinux = valueof(x);
+ else if (STREQ(y->name, "pti_init"))
+ st->pti_init_vmlinux = valueof(y);
+
if (STREQ(x->name, "saved_command_line"))
st->saved_command_line_vmlinux = valueof(x);
else if (STREQ(y->name, "saved_command_line"))
st->saved_command_line_vmlinux = valueof(y);
-
- if (STREQ(x->name, "pti_init"))
- st->pti_init_vmlinux = valueof(x);
- else if (STREQ(y->name, "kaiser_init"))
- st->kaiser_init_vmlinux = valueof(y);
}
xs = bfd_get_section(x);
--
1.8.3.1
5 a.m.
New subject: [PATCH v2 3/4] kaslr: cleanup how to set values to the arguments of calc_kaslr_offset()
From: HATAYAMA Daisuke <d.hatayama(a)jp.fujitsu.com>
Setting values of the arguments of calc_kaslr_offset() should be done
at the end of the function. Currently, they are set in the middle
where their values could still be changed according to
get_kaslr_offset_from_vmcoreinfo(). This behavior will be problematic
in the later commits when we implement a trial-and-error approach
because the value of kaslr_offset could be passed to the outside of
calc_kaslr_offset() unexpectedly. Thus, fix this first.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
kaslr_helper.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/kaslr_helper.c b/kaslr_helper.c
index fe5909c..acbb5c2 100644
--- a/kaslr_helper.c
+++ b/kaslr_helper.c
@@ -394,10 +394,11 @@ quit:
#define PTI_USER_PGTABLE_MASK (1 << PTI_USER_PGTABLE_BIT)
#define CR3_PCID_MASK 0xFFFull
int
-calc_kaslr_offset(ulong *kaslr_offset, ulong *phys_base)
+calc_kaslr_offset(ulong *ko, ulong *pb)
{
uint64_t cr3 = 0, idtr = 0, pgd = 0, idtr_paddr;
ulong divide_error_vmcore;
+ ulong kaslr_offset, phys_base;
ulong kaslr_offset_kdump, phys_base_kdump;
int ret = FALSE;
int verbose = CRASHDEBUG(1)? 1: 0;
@@ -445,9 +446,9 @@ calc_kaslr_offset(ulong *kaslr_offset, ulong *phys_base)
/* Now we can calculate kaslr_offset and phys_base */
divide_error_vmcore = get_vec0_addr(idtr_paddr);
- *kaslr_offset = divide_error_vmcore - st->divide_error_vmlinux;
- *phys_base = idtr_paddr -
- (st->idt_table_vmlinux + *kaslr_offset - __START_KERNEL_map);
+ kaslr_offset = divide_error_vmcore - st->divide_error_vmlinux;
+ phys_base = idtr_paddr -
+ (st->idt_table_vmlinux + kaslr_offset - __START_KERNEL_map);
if (CRASHDEBUG(1)) {
fprintf(fp, "calc_kaslr_offset: idtr=%lx\n", idtr);
@@ -465,9 +466,9 @@ calc_kaslr_offset(ulong *kaslr_offset, ulong *phys_base)
* from vmcoreinfo
*/
if (get_kaslr_offset_from_vmcoreinfo(
- *kaslr_offset, &kaslr_offset_kdump, &phys_base_kdump)) {
- *kaslr_offset = kaslr_offset_kdump;
- *phys_base = phys_base_kdump;
+ kaslr_offset, &kaslr_offset_kdump, &phys_base_kdump)) {
+ kaslr_offset = kaslr_offset_kdump;
+ phys_base = phys_base_kdump;
} else if (CRASHDEBUG(1)) {
fprintf(fp, "kaslr_helper: failed to determine which kernel was running at
crash,\n");
fprintf(fp, "kaslr_helper: asssuming the kdump 1st kernel.\n");
@@ -475,10 +476,13 @@ calc_kaslr_offset(ulong *kaslr_offset, ulong *phys_base)
if (CRASHDEBUG(1)) {
fprintf(fp, "calc_kaslr_offset: kaslr_offset=%lx\n",
- *kaslr_offset);
- fprintf(fp, "calc_kaslr_offset: phys_base=%lx\n", *phys_base);
+ kaslr_offset);
+ fprintf(fp, "calc_kaslr_offset: phys_base=%lx\n", phys_base);
}
+ *ko = kaslr_offset;
+ *pb = phys_base;
+
ret = TRUE;
quit:
vt->kernel_pgd[0] = 0;
--
1.8.3.1
5 a.m.
New subject: [PATCH v2 4/4] kaslr: fix failure of calculating kaslr_offset due to an sadump format restriction
From: HATAYAMA Daisuke <d.hatayama(a)jp.fujitsu.com>
We faced recently a memory dump collected by sadump where unused part
of register values are non-zero. For the crash dump, calculating
kaslr_offset fails because it is based on the assumption that unused
part of register values in the sadump format are always zero cleared.
The problem is that used and unused part of register values are
rigorously indistinguishable in the sadump format. Although there is
kernel data structure that represents a map between logical cpu
numbers and lapic ids, they cannot be used in order to calculate
kaslr_offset.
To fix this, we have no choice but use a trial-and-error approach: try
to use each entry of register values in order until we find a good
pair of cr3 and idtr by which we can refer to linux_banner symbol as
expected.
This fix is for the sadump specific issue, so there is no functional
change for the other crash dump formats.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
kaslr_helper.c | 39 +++++++++++++++++++++++++++++++++++----
sadump.c | 20 ++++++++++++--------
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/kaslr_helper.c b/kaslr_helper.c
index acbb5c2..bb19e54 100644
--- a/kaslr_helper.c
+++ b/kaslr_helper.c
@@ -406,6 +406,7 @@ calc_kaslr_offset(ulong *ko, ulong *pb)
if (!machine_type("X86_64"))
return FALSE;
+retry:
if (SADUMP_DUMPFILE()) {
if (!sadump_get_cr3_idtr(&cr3, &idtr))
return FALSE;
@@ -437,12 +438,20 @@ calc_kaslr_offset(ulong *ko, ulong *pb)
machdep->machspec->pgdir_shift = PGDIR_SHIFT;
machdep->machspec->ptrs_per_pgd = PTRS_PER_PGD;
if (!readmem(pgd, PHYSADDR, machdep->pgd, PAGESIZE(),
- "pgd", RETURN_ON_ERROR))
- goto quit;
+ "pgd", RETURN_ON_ERROR)) {
+ if (SADUMP_DUMPFILE())
+ goto retry;
+ else
+ goto quit;
+ }
/* Convert virtual address of IDT table to physical address */
- if (!kvtop(NULL, idtr, &idtr_paddr, verbose))
- goto quit;
+ if (!kvtop(NULL, idtr, &idtr_paddr, verbose)) {
+ if (SADUMP_DUMPFILE())
+ goto retry;
+ else
+ goto quit;
+ }
/* Now we can calculate kaslr_offset and phys_base */
divide_error_vmcore = get_vec0_addr(idtr_paddr);
@@ -450,6 +459,28 @@ calc_kaslr_offset(ulong *ko, ulong *pb)
phys_base = idtr_paddr -
(st->idt_table_vmlinux + kaslr_offset - __START_KERNEL_map);
+ if (SADUMP_DUMPFILE()) {
+ char buf[sizeof("Linux version")];
+ ulong linux_banner_paddr;
+
+ if (!kvtop(NULL,
+ st->linux_banner_vmlinux + kaslr_offset,
+ &linux_banner_paddr,
+ verbose))
+ goto retry;
+
+ if (!readmem(linux_banner_paddr,
+ PHYSADDR,
+ buf,
+ sizeof(buf),
+ "linux_banner",
+ RETURN_ON_ERROR))
+ goto retry;
+
+ if (!STRNEQ(buf, "Linux version"))
+ goto retry;
+ }
+
if (CRASHDEBUG(1)) {
fprintf(fp, "calc_kaslr_offset: idtr=%lx\n", idtr);
fprintf(fp, "calc_kaslr_offset: pgd=%lx\n", pgd);
diff --git a/sadump.c b/sadump.c
index 35f7cf0..26ed119 100644
--- a/sadump.c
+++ b/sadump.c
@@ -1666,21 +1666,24 @@ get_sadump_smram_cpu_state_any(struct sadump_smram_cpu_state
*smram)
{
ulong offset;
struct sadump_header *sh = sd->dump_header;
- int apicid;
- struct sadump_smram_cpu_state scs, zero;
+ static int apicid;
+ struct sadump_smram_cpu_state scs;
- offset = sd->sub_hdr_offset + sizeof(uint32_t) +
- sd->dump_header->nr_cpus * sizeof(struct sadump_apic_state);
+ if (apicid >= sh->nr_cpus)
+ return FALSE;
- memset(&zero, 0, sizeof(zero));
+ offset = sd->sub_hdr_offset + sizeof(uint32_t) +
+ sd->dump_header->nr_cpus * sizeof(struct sadump_apic_state) +
+ apicid * sizeof(scs);
- for (apicid = 0; apicid < sh->nr_cpus; ++apicid) {
+ while (apicid < sh->nr_cpus) {
+ apicid++;
if (!read_device(&scs, sizeof(scs), &offset)) {
error(INFO, "sadump: cannot read sub header "
"cpu_state\n");
return FALSE;
}
- if (memcmp(&scs, &zero, sizeof(scs)) != 0) {
+ if (scs.Cr3 && (scs.IdtUpper || scs.IdtLower)) {
*smram = scs;
return TRUE;
}
@@ -1695,7 +1698,8 @@ sadump_get_cr3_idtr(ulong *cr3, ulong *idtr)
struct sadump_smram_cpu_state scs;
memset(&scs, 0, sizeof(scs));
- get_sadump_smram_cpu_state_any(&scs);
+ if (!get_sadump_smram_cpu_state_any(&scs))
+ return FALSE;
*cr3 = scs.Cr3;
*idtr = ((uint64_t)scs.IdtUpper)<<32 | (uint64_t)scs.IdtLower;
--
1.8.3.1
7:33 p.m.
Hi Hatayama-san,
Thank you for the new series, it looks good to me.
For the v2 series,
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Bhupesh, Lianbo, could either of you review this series?
Thanks,
Kazu
-----Original Message-----
This patch series fix failure of calculating kaslr_offset due to an
sadump format restriction.
The main part is the 4-th patch. I found some small bugs during this
work and the 2nd and 3rd are to fix them and are also needed for the
4-th patch.
This issue occurs also in makedumpfile, and I have already posted the
corresponding patch for makedumpfile at:
http://lists.infradead.org/pipermail/kexec/2020-July/020852.html
ChangeLog:
v1=>v2:
- Add Signed-off-by: in each commit description
- 4/4: Remove unused zero variable in get_sadump_smram_cpu_state_any()
- 2/4: Fix mismatch of the symbol name and the member of the symbol table
HATAYAMA Daisuke (4):
symbols: Add linux_banner_vmlinux in symbol table
symbols: fix initialization of st->{pti_init,kaiser}_vmlinux
kaslr: cleanup how to set values to the arguments of
calc_kaslr_offset()
kaslr: fix failure of calculating kaslr_offset due to an sadump format
restriction
defs.h | 1 +
kaslr_helper.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------------
sadump.c | 20 +++++++++++--------
symbols.c | 25 +++++++++++++++++++-----
4 files changed, 81 insertions(+), 26 deletions(-)
--
1.8.3.1
4:51 a.m.
在 2020年07月29日 08:33, HAGIO KAZUHITO(萩尾 一仁) 写道:
Hi Hatayama-san,
Thank you for the new series, it looks good to me.
For the v2 series,
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Bhupesh, Lianbo, could either of you review this series?
Hi, Hatayama-san, Kazu
Sorry for the delay. Ack the v2 series. But, seems that it has some style issues in the
fourth patch [PATCH v2 4/4], for example:
"ERROR: code indent should use tabs where possible"
Would you mind correcting the above code style issues when merging the fourth patch? Thank
you in advance, Kazu.
BTW: we could check it with the scripts/checkpatch.pl.
Thanks.
Lianbo
Thanks,
Kazu
-----Original Message-----
> This patch series fix failure of calculating kaslr_offset due to an
> sadump format restriction.
>
> The main part is the 4-th patch. I found some small bugs during this
> work and the 2nd and 3rd are to fix them and are also needed for the
> 4-th patch.
>
> This issue occurs also in makedumpfile, and I have already posted the
> corresponding patch for makedumpfile at:
>
> http://lists.infradead.org/pipermail/kexec/2020-July/020852.html
>
> ChangeLog:
> v1=>v2:
> - Add Signed-off-by: in each commit description
> - 4/4: Remove unused zero variable in get_sadump_smram_cpu_state_any()
> - 2/4: Fix mismatch of the symbol name and the member of the symbol table
>
> HATAYAMA Daisuke (4):
> symbols: Add linux_banner_vmlinux in symbol table
> symbols: fix initialization of st->{pti_init,kaiser}_vmlinux
> kaslr: cleanup how to set values to the arguments of
> calc_kaslr_offset()
> kaslr: fix failure of calculating kaslr_offset due to an sadump format
> restriction
>
> defs.h | 1 +
> kaslr_helper.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------------
> sadump.c | 20 +++++++++++--------
> symbols.c | 25 +++++++++++++++++++-----
> 4 files changed, 81 insertions(+), 26 deletions(-)
>
> --
> 1.8.3.1
6:24 a.m.
在 2020年08月04日 17:51, lijiang 写道:
在 2020年07月29日 08:33, HAGIO KAZUHITO(萩尾 一仁) 写道:
> Hi Hatayama-san,
>
> Thank you for the new series, it looks good to me.
>
> For the v2 series,
> Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
>
> Bhupesh, Lianbo, could either of you review this series?
>
Hi, Hatayama-san, Kazu
Sorry for the delay. Ack the v2 series. But, seems that it has some style issues in the
fourth patch [PATCH v2 4/4], for example:
"ERROR: code indent should use tabs where possible"
Would you mind correcting the above code style issues when merging the fourth patch?
Thank you in advance, Kazu.
Applied. Thank you, Hatayama-san.
BTW: we could check it with the scripts/checkpatch.pl.
Thanks.
Lianbo
> Thanks,
> Kazu
>
> -----Original Message-----
>> This patch series fix failure of calculating kaslr_offset due to an
>> sadump format restriction.
>>
>> The main part is the 4-th patch. I found some small bugs during this
>> work and the 2nd and 3rd are to fix them and are also needed for the
>> 4-th patch.
>>
>> This issue occurs also in makedumpfile, and I have already posted the
>> corresponding patch for makedumpfile at:
>>
>> http://lists.infradead.org/pipermail/kexec/2020-July/020852.html
>>
>> ChangeLog:
>> v1=>v2:
>> - Add Signed-off-by: in each commit description
>> - 4/4: Remove unused zero variable in get_sadump_smram_cpu_state_any()
>> - 2/4: Fix mismatch of the symbol name and the member of the symbol table
>>
>> HATAYAMA Daisuke (4):
>> symbols: Add linux_banner_vmlinux in symbol table
>> symbols: fix initialization of st->{pti_init,kaiser}_vmlinux
>> kaslr: cleanup how to set values to the arguments of
>> calc_kaslr_offset()
>> kaslr: fix failure of calculating kaslr_offset due to an sadump format
>> restriction
>>
>> defs.h | 1 +
>> kaslr_helper.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------------
>> sadump.c | 20 +++++++++++--------
>> symbols.c | 25 +++++++++++++++++++-----
>> 4 files changed, 81 insertions(+), 26 deletions(-)
>>
>> --
>> 1.8.3.1
>
1606
days inactive
1614
days old
devel@lists.crash-utility.osci.io
8 comments
4 participants
participants (4)
-
d.hatayama＠fujitsu.com -
HAGIO KAZUHITO(萩尾 一仁) -
HATAYAMA Daisuke
-
lijiang