[PATCH] build_mem_phdrs(): check if p

Discussion:

[PATCH] build_mem_phdrs(): check if p_paddr is invalid

Pratyush Anand

2017-03-01 05:49:42 UTC

Currently, all the p_paddr of PT_LOAD headers are assigned to 0, which
is not correct and could be misleading, since 0 is a valid physical
address.

Upstream kernel commit "464920104bf7 /proc/kcore: update physical
address for kcore ram and text" fixed it and now invalid PT_LOAD is
assigned as -1.

kexec/arch/i386/crashdump-x86.c:get_kernel_vaddr_and_size() uses kcore
interface and so calls build_mem_phdrs() for kcore PT_LOAD headers.

This patch fixes build_mem_phdrs() to check if p_paddr is invalid.

Signed-off-by: Pratyush Anand <***@redhat.com>
---
kexec/kexec-elf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec-elf.c b/kexec/kexec-elf.c
index 1d6320a2f0e6..be60bbd48486 100644
--- a/kexec/kexec-elf.c
+++ b/kexec/kexec-elf.c
@@ -432,7 +432,8 @@ static int build_mem_phdrs(const char *buf, off_t len, struct mem_ehdr *ehdr,
}
return -1;
}
- if ((phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
+ if (phdr->p_paddr != (unsigned long long)-1 &&
+ (phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
/* The memory address wraps */
if (probe_debug) {
fprintf(stderr, "ELF address wrap around\n");

--
2.9.3

Dave Young

2017-03-01 07:13:17 UTC

Permalink

Post by Pratyush Anand
Currently, all the p_paddr of PT_LOAD headers are assigned to 0, which
is not correct and could be misleading, since 0 is a valid physical
address.
Upstream kernel commit "464920104bf7 /proc/kcore: update physical
address for kcore ram and text" fixed it and now invalid PT_LOAD is
assigned as -1.
kexec/arch/i386/crashdump-x86.c:get_kernel_vaddr_and_size() uses kcore
interface and so calls build_mem_phdrs() for kcore PT_LOAD headers.
This patch fixes build_mem_phdrs() to check if p_paddr is invalid.
---
kexec/kexec-elf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kexec/kexec-elf.c b/kexec/kexec-elf.c
index 1d6320a2f0e6..be60bbd48486 100644
--- a/kexec/kexec-elf.c
+++ b/kexec/kexec-elf.c
@@ -432,7 +432,8 @@ static int build_mem_phdrs(const char *buf, off_t len, struct mem_ehdr *ehdr,
}
return -1;
}
- if ((phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
+ if (phdr->p_paddr != (unsigned long long)-1 &&
+ (phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
/* The memory address wraps */
if (probe_debug) {
fprintf(stderr, "ELF address wrap around\n");
--
2.9.3

Acked-by: Dave Young <***@redhat.com>

Thanks
Dave

Pratyush Anand

2017-03-07 01:23:20 UTC

Permalink

Hi Simon,

Any comment on this?

Post by Pratyush Anand
---
kexec/kexec-elf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kexec/kexec-elf.c b/kexec/kexec-elf.c
index 1d6320a2f0e6..be60bbd48486 100644
--- a/kexec/kexec-elf.c
+++ b/kexec/kexec-elf.c
@@ -432,7 +432,8 @@ static int build_mem_phdrs(const char *buf, off_t len, struct mem_ehdr *ehdr,
}
return -1;
}
- if ((phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
+ if (phdr->p_paddr != (unsigned long long)-1 &&
+ (phdr->p_paddr + phdr->p_memsz) < phdr->p_paddr) {
/* The memory address wraps */
if (probe_debug) {
fprintf(stderr, "ELF address wrap around\n");
--
2.9.3

~Pratyush

Dave Young

2017-03-13 06:45:29 UTC

Permalink

Post by Pratyush Anand
Hi Simon,

Any comment on this?

Simon, ping, explain a bit about the background based on the patch log:

Although it is a kernel regression, the original p_addr = 0 assumption is
wrong, it should be fixed in kernel instead of keep it to avoid
kexec-tools breakage.

Since latest kernel already has the fix merged, kexec-tools will fail to
load crash kernel now.

Any opinion about this patch?

Post by Pratyush Anand

~Pratyush
_______________________________________________
kexec mailing list
http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

Simon Horman

2017-03-13 08:57:59 UTC

Permalink

Post by Dave Young

Post by Pratyush Anand
Hi Simon,

Any comment on this?

Although it is a kernel regression, the original p_addr = 0 assumption is
wrong, it should be fixed in kernel instead of keep it to avoid
kexec-tools breakage.
Since latest kernel already has the fix merged, kexec-tools will fail to
load crash kernel now.
Any opinion about this patch?

Sorry for letting this slip through the cracks.

The patch looks good to me and I have applied it.