Hi there,
I noticed that newer kernels put NT_SIGINFO and NT_FILE core notes into core dumps.
NT_SIGINFO is tricky as interpretation of fields further in the note depends of those earlier in. I don't think our core note item mechanism can handle this. Instead, I made them special, they are handled similarly to how NT_AUXV is. The patch is attached below.
In reality, the libebl/eblcorenotetypename.c is split out into a separate commit, and there are ChangeLog's. I thought this format might make this easier to review. It's all on branch pmachata/NT_SIGINFO.
I haven't had a chance to look at NT_FILE yet, but I intend to.
OK for master?
Thanks, PM
diff --git a/libebl/eblcorenotetypename.c b/libebl/eblcorenotetypename.c index 21fff73..b6db6cd 100644 --- a/libebl/eblcorenotetypename.c +++ b/libebl/eblcorenotetypename.c @@ -1,5 +1,5 @@ /* Return note type name. - Copyright (C) 2002, 2007, 2008, 2012 Red Hat, Inc. + Copyright (C) 2002, 2007, 2008, 2012, 2013 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper drepper@redhat.com, 2002.
@@ -91,6 +91,11 @@ ebl_core_note_type_name (ebl, type, buf, len) KNOWNSTYPE (S390_LAST_BREAK); KNOWNSTYPE (S390_SYSTEM_CALL); KNOWNSTYPE (ARM_VFP); + KNOWNSTYPE (ARM_TLS); + KNOWNSTYPE (ARM_HW_BREAK); + KNOWNSTYPE (ARM_HW_WATCH); + KNOWNSTYPE (SIGINFO); + KNOWNSTYPE (FILE); #undef KNOWNSTYPE
default: diff --git a/src/readelf.c b/src/readelf.c index 119c100..199199a 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -42,6 +42,7 @@ #include <unistd.h> #include <sys/param.h> #include <sys/stat.h> +#include <signal.h>
#include <system.h> #include "../libelf/libelfP.h" @@ -8617,6 +8618,93 @@ handle_auxv_note (Ebl *ebl, Elf *core, GElf_Word descsz, GElf_Off desc_pos) }
static void +handle_siginfo_note (__attribute__ ((unused)) Ebl *ebl, + Elf *core, + GElf_Word descsz, + GElf_Off desc_pos) +{ + if (descsz != 128) + return; + + Elf_Data *data = elf_getdata_rawchunk (core, desc_pos, descsz, ELF_T_BYTE); + const unsigned char *ptr = data->d_buf; + if (data == NULL) + error (EXIT_FAILURE, 0, + gettext ("cannot convert core note data: %s"), elf_errmsg (-1)); + +#define READ_INT \ + ({ \ + int val; \ + ptr = convert (core, ELF_T_WORD, 1, &val, ptr, 4); \ + val; \ + }) + +#define READ_ADDR \ + ({ \ + union \ + { \ + uint64_t u64; \ + uint32_t u32; \ + } u; \ + ptr = convert (core, ELF_T_ADDR, 1, &u, ptr, sizeof u); \ + if (gelf_fsize (core, ELF_T_ADDR, 1, EV_CURRENT) == 4) \ + u.u64 = u.u32; \ + u.u64; \ + }) + + /* Siginfo head is three ints: signal number, error number, origin + code. */ + int si_signo = READ_INT; + int si_errno = READ_INT; + int si_code = READ_INT; + + /* Next is a pointer-aligned union of structures. On 64-bit + machines, that implies a word of padding. */ + if (gelf_getclass (core) == ELFCLASS64) + ptr += 4; + + printf (" si_signo:%d, si_errno:%d, si_code:%d\n", + si_signo, si_errno, si_code); + + if (si_code <= 0) + switch (si_code) + { + case SI_ASYNCIO: + case SI_MESGQ: + case SI_TIMER: + case SI_QUEUE: + default: + break; + + case SI_USER: + { + int pid = READ_INT; + int uid = READ_INT; + printf (" sender_pid:%d, sender_uid:%d\n", pid, uid); + break; + } + } + else + switch (si_signo) + { + case SIGILL: + case SIGFPE: + case SIGSEGV: + case SIGBUS: + { + uint64_t addr = READ_ADDR; + printf (" sigfault.addr:%#" PRIx64 "\n", addr); + break; + } + default: + ; + } + +#undef READ_INT +#undef READ_ADDR +} + +static void handle_core_note (Ebl *ebl, const GElf_Nhdr *nhdr, const char *name, const void *desc) { @@ -8689,6 +8777,10 @@ handle_notes_data (Ebl *ebl, const GElf_Ehdr *ehdr, && !memcmp (name, "CORE", 4)) handle_auxv_note (ebl, ebl->elf, nhdr.n_descsz, start + desc_offset); + else if (nhdr.n_type == NT_SIGINFO + && nhdr.n_namesz == 5 && strcmp (name, "CORE") == 0) + handle_siginfo_note (ebl, ebl->elf, nhdr.n_descsz, + start + desc_offset); else handle_core_note (ebl, &nhdr, name, desc); } diff --git a/tests/Makefile.am b/tests/Makefile.am index 58db6c3..0024395 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -198,7 +198,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \ run-readelf-mixed-corenote.sh testfile63.bz2 testfile64.bz2 \ testfile65.bz2 testfile67.bz2 testfile68.bz2 \ testfile69.core.bz2 testfile69.so.bz2 \ - testfile70.core.bz2 testfile70.exec.bz2 \ + testfile70.core.bz2 testfile70.exec.bz2 testfile71.bz2 \ run-dwfllines.sh run-dwfl-report-elf-align.sh \ testfile-dwfl-report-elf-align-shlib.so.bz2 \ testfilenolines.bz2 test-core-lib.so.bz2 test-core.core.bz2 \ diff --git a/tests/run-readelf-mixed-corenote.sh b/tests/run-readelf-mixed-corenote.sh index 915bdeb..ab8a72f 100755 --- a/tests/run-readelf-mixed-corenote.sh +++ b/tests/run-readelf-mixed-corenote.sh @@ -1,5 +1,5 @@ #! /bin/sh -# Copyright (C) 2012 Red Hat, Inc. +# Copyright (C) 2012, 2013 Red Hat, Inc. # This file is part of elfutils. # # This file is free software; you can redistribute it and/or modify @@ -217,4 +217,57 @@ Note segment of 852 bytes at offset 0x94: high_r15: 0x00000000 EOF
+testfiles testfile71 +testrun_compare ${abs_top_builddir}/src/readelf -n testfile71 <<\EOF + +Note segment of 1476 bytes at offset 0x430: + Owner Data size Type + CORE 336 PRSTATUS + info.si_signo: 11, info.si_code: 0, info.si_errno: 0, cursig: 11 + sigpend: <> + sighold: <> + pid: 9664, ppid: 2868, pgrp: 9664, sid: 2868 + utime: 0.000000, stime: 0.004000, cutime: 0.000000, cstime: 0.000000 + orig_rax: -1, fpvalid: 0 + r15: 0 r14: 0 + r13: 140734971656848 r12: 4195328 + rbp: 0x00007fff69fe39b0 rbx: 0 + r11: 266286012928 r10: 140734971656256 + r9: 0 r8: 266289790592 + rax: 305419896 rcx: 4195584 + rdx: 140734971656872 rsi: 140734971656856 + rdi: 1 rip: 0x00000000004004f9 + rflags: 0x0000000000010246 rsp: 0x00007fff69fe39b0 + fs.base: 0x00007fa1c8933740 gs.base: 0x0000000000000000 + cs: 0x0033 ss: 0x002b ds: 0x0000 es: 0x0000 fs: 0x0000 gs: 0x0000 + CORE 136 PRPSINFO + state: 0, sname: R, zomb: 0, nice: 0, flag: 0x0000000000000200 + uid: 1000, gid: 1000, pid: 9664, ppid: 2868, pgrp: 9664, sid: 2868 + fname: a.out, psargs: ./a.out + CORE 128 SIGINFO + si_signo:11, si_errno:0, si_code:1 + sigfault.addr:0x12345678 + CORE 304 AUXV + SYSINFO_EHDR: 0x7fff69ffe000 + HWCAP: 0xafebfbff <fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss tm pbe> + PAGESZ: 4096 + CLKTCK: 100 + PHDR: 0x400040 + PHENT: 56 + PHNUM: 9 + BASE: 0 + FLAGS: 0 + ENTRY: 0x400400 + UID: 1000 + EUID: 1000 + GID: 1000 + EGID: 1000 + SECURE: 0 + RANDOM: 0x7fff69fe3d19 + EXECFN: 0x7fff69fe4ff0 + PLATFORM: 0x7fff69fe3d29 + NULL + CORE 469 FILE +EOF + exit 0 diff --git a/tests/testfile71.bz2 b/tests/testfile71.bz2 new file mode 100644 index 0000000..ce5b08f Binary files /dev/null and b/tests/testfile71.bz2 differ
Petr Machata pmachata@redhat.com writes:
I haven't had a chance to look at NT_FILE yet, but I intend to.
pmachata/NT_SIGINFO now includes code for handling NT_FILE as well. I changed the READ_INT etc. macros to functions (they can be reused for NT_FILE) and rewrote the branch history with this change in.
Jan mentioned in his e-mails earlier this year that NT_FILE could be used to prime dwfl_link_map_report, which would help in cases where link_map chain gets corrupted. I haven't had a chance to look into this yet. We'd move the NT_FILE code from readelf to some library in that case.
Thanks, PM
On Thu, 2013-09-26 at 21:41 +0200, Petr Machata wrote:
Petr Machata pmachata@redhat.com writes:
I haven't had a chance to look at NT_FILE yet, but I intend to.
pmachata/NT_SIGINFO now includes code for handling NT_FILE as well. I changed the READ_INT etc. macros to functions (they can be reused for NT_FILE) and rewrote the branch history with this change in.
I do prefer reviewing patches on the list. You can easily sent patches from the branch to the list for review with something like: $ git send-email --to=elfutils-devel@lists.fedorahosted.org \ master..pmachata/NT_SIGINFO
Looking at the branch:
commit 49a1a381a98f0bbe13014d572596055c90ef4b99 Author: Petr Machata pmachata@redhat.com Date: Mon Jun 17 23:52:27 2013 +0200
Update elf.h from glibc
Signed-off-by: Petr Machata pmachata@redhat.com
+2013-06-17 Petr Machata pmachata@redhat.com
* elf.h: Update from glibc.
This is fine.
commit 767633cf8b5bdc4b8a733b958e59e6750b40e33f Author: Petr Machata pmachata@redhat.com Date: Thu Sep 26 00:38:37 2013 +0200
Recognize names of some new core note types in ebl_core_note_type_name
Signed-off-by: Petr Machata pmachata@redhat.com
+2013-09-26 Petr Machata pmachata@redhat.com
* eblcorenotetypename.c: Handle NT_ARM_TLS, NT_ARM_HW_BREAK,
NT_ARM_HW_WATCH, NT_SIGINFO, NT_FILE.
Looks fine too.
commit c5fce1f17fae87f07a3f880ced819c6fe7987fcc Author: Petr Machata pmachata@redhat.com Date: Thu Sep 26 00:39:34 2013 +0200
Show contents NT_SIGINFO core note in readelf
Signed-off-by: Petr Machata pmachata@redhat.com
+static int +buf_has_data (unsigned char const *ptr, unsigned char const *end, size_t sz) +{
- return ptr < end && (size_t)(end - ptr) >= sz;
^ Missing space after closing bracket.
+static int +buf_read_int (Elf *core, unsigned char const **ptrp, unsigned char const *end) +{
- if (!buf_has_data (*ptrp, end, 4))
^
I like a space after the ! to make the negation to stand out. But admittedly the file isn't consistent.
- return printf ("<buffer overrun>"), 0;
ehe, OK, I admit I didn't know you could do that with a return statement. I assume it returns zero?
I would have added int *val as argument and let the function return a boolean or int to signal failure to read the int value. Then in the caller (handle_siginfo_note) just check the return value of buf_read_int and print the error there. Then you can also abort early instead of keeping printing error messages.
+static uint64_t +buf_read_ulong (Elf *core, unsigned char const **ptrp, unsigned char const *end)
Same comment as above.
+static void +handle_siginfo_note (Elf *core, GElf_Word descsz, GElf_Off desc_pos) +{
- if (descsz != 128)
- return;
The magic value needs a comment. Should it be < 128 (to allow expansion of the note content later?) Also don't you want to print some error/unexpected message here?
- /* Siginfo head is three ints: signal number, error number, origin
code. */
- int si_signo = buf_read_int (core, &ptr, end);
- int si_errno = buf_read_int (core, &ptr, end);
- int si_code = buf_read_int (core, &ptr, end);
With the suggestion above this would become:
int si_signo, si_errno, si_code; if (! buf_read_int (core, &ptr, end, &si_signo) || ! buf_read_int (core, &ptr, end, &si_errno) || ! buf_read_int (core, &ptr, end, &si_code)) { printf ("<buffer overrun>"); return; }
- printf (" si_signo:%d, si_errno:%d, si_code:%d\n",
si_signo, si_errno, si_code);
Does it make sense to translate the si_code and si_node into something human readable in the switch case below?
+2013-09-26 Petr Machata pmachata@redhat.com
* Makefile.am (EXTRA_DIST): Add testfile71.bz2.
* run-readelf-mixed-corenote.sh: New test for this file.
* testfile71.bz2: New file.
Looks fine. But I would like a little note in run-readelf-mixed-corenote.sh how the test file was created just in case someone wants to replicate it later in a different environment.
commit 648831244b406446c726584cd09e131f561bc444 Author: Petr Machata pmachata@redhat.com Date: Thu Sep 26 21:02:22 2013 +0200
Show contents NT_FILE core note in readelf
Signed-off-by: Petr Machata pmachata@redhat.com
2013-09-26 Petr Machata pmachata@redhat.com
* readelf.c (handle_file_note): New function.
(handle_notes_data): Call it to handle NT_FILE notes.
- uint64_t count = buf_read_ulong (core, &ptr, end);
- uint64_t page_size = buf_read_ulong (core, &ptr, end);
Depending on whether you take the suggestion on the previous patch, these would change a little. There are a couple more.
- char const *fptr = (char *)fstart;
^ Missing space.
const char *fnext = memchr (fptr, '\0', (char *)end - fptr);
^
Likewise.
2013-09-26 Petr Machata pmachata@redhat.com
* run-readelf-mixed-corenote.sh: Update output of testfile71
dump--readelf can newly decode the NT_FILE note.
Looks fine.
Thanks,
Mark
Mark Wielaard mjw@redhat.com writes:
- return printf ("<buffer overrun>"), 0;
ehe, OK, I admit I didn't know you could do that with a return statement. I assume it returns zero?
Yeah, that's just return EXPRESSION where the expression involves a comma operator. Nothing magic about it. I admit this bit slipped my attention, I didn't mean to push it. It's not exactly an example of stylistical purity.
I would have added int *val as argument and let the function return a boolean or int to signal failure to read the int value.
Yeah, I didn't like how verbose it all became, but let's have it this way.
- if (descsz != 128)
- return;
The magic value needs a comment. Should it be < 128 (to allow expansion of the note content later?) Also don't you want to print some error/unexpected message here?
All NT_SIGINFO notes are 128 bytes in size. If it isn't 128 bytes, it's not NT_SIGINFO. That's the logic employed by the generic note item decoder as well--if the length of the note doesn't match what we think it should be, we don't decode.
But that makes sense for the item decoder, where you then proceed to read random offsets supplied by the EBL backend. Here, we have our own size checks. So I just dropped the condition.
- printf (" si_signo:%d, si_errno:%d, si_code:%d\n",
si_signo, si_errno, si_code);
Does it make sense to translate the si_code and si_node into something human readable in the switch case below?
This is as human readable as it gets. PRSTATUS mentions these variables as well. I see value in humanizing the other messages though: changed them to "fault address", "sender PID" and "sender UID".
I'll post the updated patch set later today.
Thanks, PM
elfutils-devel@lists.fedorahosted.org