* [PATCH] build_mem_phdrs(): check if p_paddr is invalid
@ 2017-03-01 5:49 Pratyush Anand
2017-03-01 7:13 ` Dave Young
0 siblings, 1 reply; 5+ messages in thread
From: Pratyush Anand @ 2017-03-01 5:49 UTC (permalink / raw)
To: horms; +Cc: Pratyush Anand, dyoung, kexec, bhe
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 <panand@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
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] build_mem_phdrs(): check if p_paddr is invalid
2017-03-01 5:49 [PATCH] build_mem_phdrs(): check if p_paddr is invalid Pratyush Anand
@ 2017-03-01 7:13 ` Dave Young
2017-03-07 1:23 ` Pratyush Anand
0 siblings, 1 reply; 5+ messages in thread
From: Dave Young @ 2017-03-01 7:13 UTC (permalink / raw)
To: Pratyush Anand; +Cc: horms, kexec, bhe
On 03/01/17 at 11:19am, Pratyush Anand wrote:
> 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 <panand@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
>
Acked-by: Dave Young <dyoung@redhat.com>
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] build_mem_phdrs(): check if p_paddr is invalid
2017-03-01 7:13 ` Dave Young
@ 2017-03-07 1:23 ` Pratyush Anand
2017-03-13 6:45 ` Dave Young
0 siblings, 1 reply; 5+ messages in thread
From: Pratyush Anand @ 2017-03-07 1:23 UTC (permalink / raw)
To: horms; +Cc: kexec, Dave Young, bhe
Hi Simon,
On Wednesday 01 March 2017 12:43 PM, Dave Young wrote:
> On 03/01/17 at 11:19am, Pratyush Anand wrote:
>> 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.
Any comment on this?
>>
>> Signed-off-by: Pratyush Anand <panand@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
>>
>
> Acked-by: Dave Young <dyoung@redhat.com>
>
~Pratyush
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] build_mem_phdrs(): check if p_paddr is invalid
2017-03-07 1:23 ` Pratyush Anand
@ 2017-03-13 6:45 ` Dave Young
2017-03-13 8:57 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: Dave Young @ 2017-03-13 6:45 UTC (permalink / raw)
To: Pratyush Anand; +Cc: horms, kexec, bhe
On 03/07/17 at 06:53am, Pratyush Anand wrote:
> Hi Simon,
>
> On Wednesday 01 March 2017 12:43 PM, Dave Young wrote:
> > On 03/01/17 at 11:19am, Pratyush Anand wrote:
> > > 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.
>
> 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?
>
> > >
> > > Signed-off-by: Pratyush Anand <panand@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
> > >
> >
> > Acked-by: Dave Young <dyoung@redhat.com>
> >
>
>
>
> ~Pratyush
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] build_mem_phdrs(): check if p_paddr is invalid
2017-03-13 6:45 ` Dave Young
@ 2017-03-13 8:57 ` Simon Horman
0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2017-03-13 8:57 UTC (permalink / raw)
To: Dave Young; +Cc: Pratyush Anand, kexec, bhe
On Mon, Mar 13, 2017 at 02:45:29PM +0800, Dave Young wrote:
> On 03/07/17 at 06:53am, Pratyush Anand wrote:
> > Hi Simon,
> >
> > On Wednesday 01 March 2017 12:43 PM, Dave Young wrote:
> > > On 03/01/17 at 11:19am, Pratyush Anand wrote:
> > > > 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.
> >
> > 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?
Sorry for letting this slip through the cracks.
The patch looks good to me and I have applied it.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-13 8:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 5:49 [PATCH] build_mem_phdrs(): check if p_paddr is invalid Pratyush Anand
2017-03-01 7:13 ` Dave Young
2017-03-07 1:23 ` Pratyush Anand
2017-03-13 6:45 ` Dave Young
2017-03-13 8:57 ` Simon Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.