| Message ID | E1axXSF-0004hH-Oo@e0050434b2927.dyn.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 3, 2016 at 3:51 PM, Russell King <rmk@arm.linux.org.uk> wrote: > When kdump tries to map the program header, it fails to take account > of ehdr->e_phoff being an offset from the start of the ELF "file", > which causes: > > Cannot mmap /dev/mem offset: 64 size: 392: Invalid argument > > Ensure that we take account of the start address when mapping this. > > This fix has been extracted from a larger patch by Vitaly Andrianov > adding support for Keystone 2. > > Signed-off-by: Russell King <rmk@arm.linux.org.uk> > --- > kdump/kdump.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kdump/kdump.c b/kdump/kdump.c > index 1f5b984..34d2149 100644 > --- a/kdump/kdump.c > +++ b/kdump/kdump.c > @@ -284,7 +284,8 @@ int main(int argc, char **argv) > } > > /* Get the program header */ > - phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), ehdr->e_phoff); > + phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), > + start_addr + ehdr->e_phoff); This is fine. But at the same time should n't we also fix the offset for mmap of memory segments? For memory segments, offset is phdr[i].p_offset, and I do not see generate_new_headers() taking care of start_addr. > > /* Collect up the notes */ > note_bytes = 0; > -- > 1.9.1 > ~Pratyush
On Wed, May 25, 2016 at 11:47:33AM +0530, Pratyush Anand wrote: > On Tue, May 3, 2016 at 3:51 PM, Russell King <rmk@arm.linux.org.uk> wrote: > > diff --git a/kdump/kdump.c b/kdump/kdump.c > > index 1f5b984..34d2149 100644 > > --- a/kdump/kdump.c > > +++ b/kdump/kdump.c > > @@ -284,7 +284,8 @@ int main(int argc, char **argv) > > } > > > > /* Get the program header */ > > - phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), ehdr->e_phoff); > > + phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), > > + start_addr + ehdr->e_phoff); > > This is fine. But at the same time should n't we also fix the offset > for mmap of memory segments? For memory segments, offset is > phdr[i].p_offset, and I do not see generate_new_headers() taking care > of start_addr. Unfortunately not. The reason is, p_offset is not an offset, but an absolute address - see kexec/crashdump-elf.c, which is the bit of code which creates the table and writes it into kernel memory when loading the panic kernel: phdr->p_offset = phdr->p_paddr = notes_addr; phdr->p_offset = phdr->p_paddr = vmcoreinfo_addr; phdr->p_offset = phdr->p_paddr = elf_info->kern_paddr_start; mstart = range->start; phdr->p_offset = mstart; phdr->p_paddr = mstart; etc. So, p_offset is also the physical address, not the file offset. Of course, that could be a bug in crashdump-elf.c. To change that, we would also need to fix crashdump-elf.c in lock-step as well.
diff --git a/kdump/kdump.c b/kdump/kdump.c index 1f5b984..34d2149 100644 --- a/kdump/kdump.c +++ b/kdump/kdump.c @@ -284,7 +284,8 @@ int main(int argc, char **argv) } /* Get the program header */ - phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), ehdr->e_phoff); + phdr = map_addr(fd, sizeof(*phdr)*(ehdr->e_phnum), + start_addr + ehdr->e_phoff); /* Collect up the notes */ note_bytes = 0;
When kdump tries to map the program header, it fails to take account of ehdr->e_phoff being an offset from the start of the ELF "file", which causes: Cannot mmap /dev/mem offset: 64 size: 392: Invalid argument Ensure that we take account of the start address when mapping this. This fix has been extracted from a larger patch by Vitaly Andrianov adding support for Keystone 2. Signed-off-by: Russell King <rmk@arm.linux.org.uk> --- kdump/kdump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)