Re: [PATCH v2 0/2] Fix KASLR problem on sadump

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

 



Hi Takao,

FYI, a few more compiler issues when compiling for the other architectures:

  $ make
  ... [ cut ] ...
  cc -c -g -DPPC64 -DLZO -DSNAPPY -DGDB_7_6  sadump.c  
  sadump.c: In function 'sadump_calc_kaslr_offset':
  sadump.c:2056:19: error: 'struct machine_specific' has no member named 'last_pml4_read'
    machdep->machspec->last_pml4_read = vt->kernel_pgd[0];
                     ^
  sadump.c:2057:19: error: 'struct machine_specific' has no member named 'physical_mask_shift'
    machdep->machspec->physical_mask_shift = __PHYSICAL_MASK_SHIFT_2_6;
                     ^
  sadump.c:2057:43: error: '__PHYSICAL_MASK_SHIFT_2_6' undeclared (first use in this function)
    machdep->machspec->physical_mask_shift = __PHYSICAL_MASK_SHIFT_2_6;
                                             ^
  sadump.c:2057:43: note: each undeclared identifier is reported only once for each function it appears in
  sadump.c:2058:19: error: 'struct machine_specific' has no member named 'pgdir_shift'
    machdep->machspec->pgdir_shift = PGDIR_SHIFT;
                     ^
  sadump.c:2059:47: error: 'struct machine_specific' has no member named 'pml4'
    if (!readmem(cr3, PHYSADDR, machdep->machspec->pml4, PAGESIZE(),
                                                 ^
  sadump.c:2071:44: error: '__START_KERNEL_map' undeclared (first use in this function)
     (st->idt_table_vmlinux + *kaslr_offset - __START_KERNEL_map);
                                              ^
  sadump.c:2104:19: error: 'struct machine_specific' has no member named 'last_pml4_read'
    machdep->machspec->last_pml4_read = 0;
                     ^
  make[4]: *** [sadump.o] Error 1
  make[3]: *** [gdb] Error 2
  make[2]: *** [rebuild] Error 2
  make[1]: *** [gdb_merge] Error 2
  make: *** [all] Error 2
  $ 

Thanks,
  Dave


----- Original Message -----
> Hi Dave,
> 
> Thanks for your comments.
> 
> On Mon, Oct 16, 2017 at 04:56:01PM -0400, Dave Anderson wrote:
> > 
> > Hi Takao,
> > 
> > A couple things.  Can you post against the current github sources,
> > or at least use crash-7.2.0?  I'm not sure what version you're
> > working with:
> >   
> >   $ patch -p1 --dry-run < $dl/sadump_1.patch
> >   checking file defs.h
> >   Hunk #1 succeeded at 5629 (offset 7 lines).
> >   checking file x86_64.c
> >   Hunk #1 FAILED at 119.
> >   Hunk #2 succeeded at 1998 (offset 34 lines).
> >   Hunk #3 succeeded at 2016 (offset 34 lines).
> >   Hunk #4 succeeded at 2030 (offset 34 lines).
> >   Hunk #5 succeeded at 2086 (offset 34 lines).
> >   1 out of 5 hunks FAILED
> >   $ patch -p1 --dry-run < $dl/sadump_2.patch
> >   checking file defs.h
> >   Hunk #1 succeeded at 2590 (offset 5 lines).
> >   Hunk #2 succeeded at 6312 (offset 47 lines).
> >   checking file sadump.c
> >   checking file sadump.h
> >   checking file symbols.c
> >   Hunk #3 succeeded at 12262 (offset 10 lines).
> >   $
> >    
> > It was easy enough to address the FAILED segment above,
> 
> It seems that I made patches on old branch :-(
> I'll rebase them.
> 
> > but there are a few issues with the sadump.c patch:
> >   
> >   $ make warn
> >   ... [ cut ] ...
> >   cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  sadump.c -Wall -O2
> >   -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> >   -Wformat-security
> >   sadump.c:1911:1: warning: no previous prototype for
> >   ‘get_kaslr_offset_from_vmcoreinfo’ [-Wmissing-prototypes]
> >    get_kaslr_offset_from_vmcoreinfo(ulong cr3, ulong orig_kaslr_offset,
> >    ^
> >   sadump.c: In function ‘sadump_calc_kaslr_offset’:
> >   sadump.c:2088:38: warning: ‘scs.Cr3’ may be used uninitialized in this
> >   function [-Wmaybe-uninitialized]
> >     if (get_kaslr_offset_from_vmcoreinfo(
> >                                         ^
> >   sadump.c:2044:40: warning: ‘scs.IdtLower’ may be used uninitialized in
> >   this function [-Wmaybe-uninitialized]
> >     idtr = ((uint64_t)scs.IdtUpper)<<32 | (uint64_t)scs.IdtLower;
> >                                           ^
> >   sadump.c:2044:10: warning: ‘scs.IdtUpper’ may be used uninitialized in
> >   this function [-Wmaybe-uninitialized]
> >     idtr = ((uint64_t)scs.IdtUpper)<<32 | (uint64_t)scs.IdtLower;
> >   ...
> > 
> 
> Will fix them in next version.
> 
> > But more importantly, the sadump.c patch won't compile against the
> > other architectures, because x86_64_kvtop_pagetable() only exists
> > when crash is built for x86_64.
> > 
> > Plus I don't particularly like the kvtop "pagetable" argument abstraction
> > and new functions that you put in place in x86_64.c just for these 3 rather
> > arcane calls from sadump.c.
> > 
> > I suggest adding a new machdep->flags bit (USE_PT?) which could
> > be temporarily set in sadump.c prior to each of the 3 calls that
> > could be made to the generic kvtop() function.  Then in x86_64_kvtop(),
> > check for the new machdep->flag as opposed to the "use_pagetable" argument.
> > It may even be possible to use SADUMP() along with some other existing
> > flag instead of creating a new one.  But please just avoid creating the
> > new functions in x86_64.c.
> 
> Ok, I'll add UST_PT or something and update patches.
> 
> Thanks,
> Takao Indoh
> 
> > 
> > Thanks,
> >   Dave
> > 
> > 
> > 
> > ----- Original Message -----
> > > Hi Dave, Hatayama-san,
> > > 
> > > Hatayama-san, thanks for your review, I updated may patch.
> > > 
> > > These patch series fix a problem that crash cannot open a dumpfile which
> > > is
> > > captured by sadump on KASLR enabled kernel.
> > > 
> > > When KASLR feature is enabled, a kernel is placed on the memory randomly
> > > and
> > > therefore crash cannot open a dumpfile because addresses of kernel
> > > symbols in
> > > vmlinux are different from actual addresses. In the case of kdump,
> > > information
> > > to get actual address is included in the vmcoreinfo, but dumpfile of
> > > sadump
> > > does
> > > not have such a information.
> > > 
> > > These patches calculate kaslr offset and phys_base to solve this problem.
> > > The
> > > basic idea is getting register (IDTR and CR3) from dump header, and
> > > calculate
> > > kaslr_offset/phys_base using them.
> > > 
> > > changelog:
> > > v2:
> > > - Remove virsh-dump part
> > > - Change get_vec0_addr style
> > > - Some tiny fixes
> > > 
> > > v1:
> > > https://www.redhat.com/archives/crash-utility/2017-October/msg00004.html
> > > 
> > > Takao Indoh (2):
> > >   Introduce x86_64_kvtop_pagetable
> > >   Fix a KASLR problem of sadump
> > > 
> > >  defs.h    |   5 +
> > >  sadump.c  | 458
> > >  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  sadump.h  |   1 +
> > >  symbols.c |  34 +++++
> > >  x86_64.c  |  26 +++-
> > >  5 files changed, 522 insertions(+), 2 deletions(-)
> > > 
> > > --
> > > 2.9.5
> > > 
> > > 
> > > --
> > > Crash-utility mailing list
> > > Crash-utility@xxxxxxxxxx
> > > https://www.redhat.com/mailman/listinfo/crash-utility
> > > 
> > 
> > --
> > Crash-utility mailing list
> > Crash-utility@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/crash-utility
> 
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
> 

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux