[Crash-utility] [PATCH] Fix for some command-line cases an unintended unlink of a file crash didn't create and a leftover temporary

[Dave Anderson]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > When the crash command line includes two filenames where one is a
> > compressed kernel and the other is a debuginfo file and they appear in
> > that order then if the uncompressed temporary version of the kernel is
> > actually larger than the debuginfo then crash will end with an error but
> > will also unlink the debuginfo file and will not clean up the (intended
> > temporary) uncompressed copy of the kernel.
> 
> Hi Dave,
> 
> It's taken me awhile to wrap my head around this, but my first question is:
> how is it possible that the uncompressed stripped kernel can possibly be
> larger than the debuginfo file?
> 
> I only have old RHEL3 kernels as examples, but the .debug versions
> of the kernel are 5 to 6 times larger than the stripped kernel.
> 
> > This patch at least fixes the unintended unlink and leaving the
> > temporary present. It doesn't fix the failure to start but that's
> > because the wrong files are assumed the debuginfo and kernel. The size
> > case that led to this discovery is probably rare.
> 
> Is the unintended unlink() done in the first display_sys_stats() or
> in clean_exit()?
> 
> And then with your fix applied, why does the crash session still fail?
> 
> Dave

Hi Dave,

Although I'm still interested in answers to the questions above, this
patch also fixes a different scenario.

In testing the 8 possible combinations using RHEL3 vmlinux/vmlinux.gz
and vmlinux.debug/vmlinux.debug.gz files, I found that I could reproduce
the problem with this particular combination/order:

 $ crash vmlinux.debug.gz vmlinux ...

Similar to your setup, the pc->namelist_orig field continues to 
be non-NULL, and therefore pc->namelist (vmlinux) gets removed in
clean_exit(), and since pc->namelist_debug_orig doesn't get set,
the uncompressed temporary debug file remains!

Nice fix!

Dave



> 
>  
> > The cause is that evidence of a temporary file to unlink is that there
> > is a value in pc->namelist and pc->namelist_orig (or pc->namelist_debug
> > and pc->namelist_orig_debug) but when the file size test in
> > select_namelist() results in the pc->namelist copy to pc->namelist_debug
> > the _orig is not copied as well so the implied file to unlink is the one
> > set in pc->namelist (the debuginfo filename now).
> > 
> > The patch causes a populated namelist_orig value to be swapped with
> > namelist_debug_orig if the namelist value is copied to namelist_debug.
> > 
> >     Signed-off-by: David Mair <dmair at suse.com>
> > ---
> >  symbols.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/symbols.c b/symbols.c
> > index 4c6fbf4..e1ed719 100644
> > --- a/symbols.c
> > +++ b/symbols.c
> > @@ -3520,6 +3520,7 @@ int
> >  select_namelist(char *new)
> >  {
> >         struct stat stat1, stat2;
> > +       char *namecp;
> > 
> >         if (pc->server_namelist) {
> >                 pc->namelist_debug = new;
> > @@ -3533,6 +3534,12 @@ select_namelist(char *new)
> > 
> >         if (stat1.st_size > stat2.st_size) {
> >                 pc->namelist_debug = pc->namelist;
> > +               if (pc->namelist_orig)
> > +               {
> > +                       namecp = pc->namelist_debug_orig;
> > +                       pc->namelist_debug_orig = pc->namelist_orig;
> > +                       pc->namelist_orig = namecp;
> > +               }
> >                 pc->namelist = new;
> >         } else if (stat2.st_size > stat1.st_size)
> >                 pc->namelist_debug = new;
> > 
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> > 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>