[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Crash-utility] add support for incomplete elf dump file




----- Original Message -----
> Hello Dave,
> 
> On 10/22/2014 09:49 PM, Dave Anderson wrote:
> >
> > Hello Wenjian,
> >
> > First -- please send patches as email attachments.  Even if I cut-and-paste
> > your
> > patch from the crash-utility archives (which normally works), there are
> > still no tabs in your patch.
> 
> Appreciate your suggestion.
> 
> > Now, with respect to the patch, it's not clear to me what would happen if you
> > make no changes to check_dumpfile_size()?
> >
> > It seems to me that a read error would occur regardless whether you change
> > the PT_LOAD-related contents or not.
> >
> > If no changes are made:
> >
> >    If a readmem() of a truncated page is attempted, read_netdump() would
> >    calculate an offset based upon the original PT_LOAD contents, but then
> >    the subsequent read() would fail, and would return a READ_ERROR.
> >
> > If your patch is applied:
> >
> >    If a readmem() of a truncated page is attempted, read_netdump() would not
> >    be able to calculate an offset, and would return a READ_ERROR.
> >
> > What's the difference?  Why even bother making the changes?
> >
> If my patch is applied:
> 	If a readmem() of a truncated page is attempted,
> 
> 	(pls->zero_fill && (paddr >= pls->phys_end) && (paddr < pls->zero_fill)),
> 	,his will be right. So read_netdump() will fill bufptr with zero and
> 	return cnt.
> 
> In my patch, information of PT_LOAD segment is modified, so that the segment will
> not go beyond the file size. And use zero to replace the lost part.
> 
> Previously, we intend to do this work in makedumpfile by modifying the PT_LOAD
> header of the ELF dump file. But kumagai atsushi thought it's not good to make the
> irreversible change. So we think do it in crash maybe a better choice.
> 
> > I also don't understand what the difference between "truncated" and "incomplete"
> > is?  Why did you separate the messages into two?
> >
> At first, I thought the difference is the incomplete flag.
> Now, I think it's not necessary to distinguish them.
> 
> 
> > Anyway, I had already created a patch in preparation for the changes to
> > makedumpfile for ELF and compressed kdump vmcores.  The patch will apply
> > to the current github master branch.  Please apply it alone, and tell me
> > what happens.
> 
> The patch can show the incomplete information, but will be interrupted by the
> READ_ERROR (we don't change the PT_LOAD headers in makedumpfile any more).

Well, it *should* be interrupted.  What's worse, "interruption" or the receipt
of completely bogus zero-filled data?

This whole scheme is even crazier than I thought.  I'm going to have
to make the initialization-time warning message even more ominous than
I already have it.

But I still don't think it's necessary to modify the original PT_LOAD
contents.  If read_netdump() finds a legitimate offset, but that offset
is beyond the end of the incomplete/truncated file, it can just check the
INCOMPLETE flag and return a zero-filled buffer if it's set.   Wouldn't
that work?

Dave



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]