Re: [Crash-utility] [Patch 4/4][crash] Recognise elf-note of type NT_NOCOREDUMP before vmcore analysis
On Mon, Oct 03, 2011 at 11:18:46AM -0400, Dave Anderson wrote:
----- Original Message -----
> The kernel might have added a new elf-note of type NT_NOCOREDUMP for various
> reasons. This patch teaches crash tool to look for the same inside a vmcore
> before further analysis. If present, display the error description and exit
> early.
>
> Signed-off-by: K.Prasad <prasad(a)linux.vnet.ibm.com>
At this point, I'll admit I'm not sure I totally understand
this patch or what the dumpfile header layout would look like.
Your new "myload64" pointer is not pointing to a PT_LOAD, but
rather the first PT_NOTE, so its name doesn't even make sense
in that respect. And for that matter, I don't see why you
didn't just use the currently existing nd->notes64 pointer,
which points to the same place? Also, the re-definition of the
currently-existing "size" value scares the hell out of me
w/respect to backwards-compatibility. And lastly, if I'm not
mistaken, when you do the realloc() of tmp_elf_header, it may
return a different address -- so wouldn't nd->elf_header be
left pointing to the old buffer? And by extension, stale pointer
values could be left in nd->elf64, nd->num_pt_load_segments,
nd->notes64, nd->load64, etc?
I agree that "myload64" variable name could have been more meaningful,
and during development nd->notes64 did not point correctly to the notes
section (for some reason that I did not investigate further) and was
forced to use a second variable.
Like you've mentioned below, many of these issues should go away by
stashing these operations into a new function check_nocoredump() invoked
from is_netdump().
I'll send out a patch with these changes once there's more clarity about
the acceptance of changes to kernel and makedumpfile.
Thanks for the review.
I would rather that you is_netdump() is left intact -- except for
a call to a new "check_nocoredump()" function, one which does not
tinker with the is_netdump() pointers, sizes, buffers, etc...
Let that function do its own thing, and if it finds that there's
no coredump, then it's not going to return and we're done. But
in 99.99% of the time, there will be a coredump, and your function
will not have screwed around with any of is_netdump()'s bookkeeping.
Anyway, when the feature is accepted upstream in the kernel,
and then by makedumpfile, we'll revisit this.
Thanks,
Dave
-- K.Prasad