Messages in this thread | | | Subject | Re: [PATCH v2][RFC] kdump: append kernel build-id string to VMCOREINFO | From | Vijay Balakrishna <> | Date | Wed, 17 Jun 2020 14:59:18 -0700 |
| |
Thanks Andrew. See inline.
On 6/17/2020 12:45 PM, Andrew Morton wrote: > On Wed, 10 Jun 2020 21:27:52 -0700 Vijay Balakrishna <vijayb@linux.microsoft.com> wrote: > >> Make kernel GNU build-id available in VMCOREINFO. Having >> build-id in VMCOREINFO facilitates presenting appropriate kernel >> namelist image with debug information file to kernel crash dump >> analysis tools. Currently VMCOREINFO lacks uniquely identifiable >> key for crash analysis automation. >> >> Regarding if this patch is necessary or matching of linux_banner >> and OSRELEASE in VMCOREINFO employed by crash(8) meets the >> need -- IMO, build-id approach more foolproof, in most instances it >> is a cryptographic hash generated using internal code/ELF bits unlike >> kernel version string upon which linux_banner is based that is >> external to the code. I feel each is intended for a different purpose. >> Also OSRELEASE is not suitable when two different kernel builds >> from same version with different features enabled. >> >> Currently for most linux (and non-linux) systems build-id can be >> extracted using standard methods for file types such as user mode crash >> dumps, shared libraries, loadable kernel modules etc., This is an >> exception for linux kernel dump. Having build-id in VMCOREINFO brings >> some uniformity for automation tools. >> >> ... >> >> --- a/kernel/crash_core.c >> +++ b/kernel/crash_core.c >> @@ -11,6 +11,8 @@ >> #include <asm/page.h> >> #include <asm/sections.h> >> >> +#include <crypto/sha.h> >> + >> /* vmcoreinfo stuff */ >> unsigned char *vmcoreinfo_data; >> size_t vmcoreinfo_size; >> @@ -376,6 +378,53 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void) >> } >> EXPORT_SYMBOL(paddr_vmcoreinfo_note); >> >> +#define NOTES_SIZE (&__stop_notes - &__start_notes) >> +#define BUILD_ID_MAX SHA1_DIGEST_SIZE >> +#define NT_GNU_BUILD_ID 3 >> + >> +struct elf_note_section { >> + struct elf_note n_hdr; >> + u8 n_data[]; >> +}; >> + >> +/* >> + * Add build ID from .notes section as generated by the GNU ld(1) >> + * or LLVM lld(1) --build-id option. >> + */ >> +static void add_build_id_vmcoreinfo(void) >> +{ >> + char build_id[BUILD_ID_MAX * 2 + 1]; >> + int n_remain = NOTES_SIZE; >> + >> + while (n_remain >= sizeof(struct elf_note)) { >> + const struct elf_note_section *note_sec = >> + &__start_notes + NOTES_SIZE - n_remain; >> + const u32 n_namesz = note_sec->n_hdr.n_namesz; >> + >> + if (note_sec->n_hdr.n_type == NT_GNU_BUILD_ID && >> + n_namesz != 0 && >> + !strcmp((char *)¬e_sec->n_data[0], "GNU")) { > > Is it guaranteed that n_data[] is null-terminated?
From ELF(5) "Notes (Nhdr)" section:
> n_namesz The length of the name field in bytes. The contents will immediately follow this note in memory. The name is null terminated. For > example, if the name is "GNU", then n_namesz will be set to 4.
Vijay
> >> + if (note_sec->n_hdr.n_descsz <= BUILD_ID_MAX) { >> + const u32 n_descsz = note_sec->n_hdr.n_descsz; >> + const u8 *s = ¬e_sec->n_data[n_namesz]; >> + >> + s = PTR_ALIGN(s, 4); >> + bin2hex(build_id, s, n_descsz); >> + build_id[2 * n_descsz] = '\0'; >> + VMCOREINFO_BUILD_ID(build_id); >> + return; >> + } >> + pr_warn("Build ID is too large to include in vmcoreinfo: %u > %u\n", >> + note_sec->n_hdr.n_descsz, >> + BUILD_ID_MAX); >> + return; >> + } >> + n_remain -= sizeof(struct elf_note) + >> + ALIGN(note_sec->n_hdr.n_namesz, 4) + >> + ALIGN(note_sec->n_hdr.n_descsz, 4); >> + } >> +} >> + >> static int __init crash_save_vmcoreinfo_init(void) >> { >> vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL);
| |