* [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2018-12-28 1:12 ` Dave Young
0 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2018-12-28 1:12 UTC (permalink / raw)
To: linux-kernel, kexec
Cc: AKASHI Takahiro, Andrew Morton, Eric W. Biederman, Baoquan He,
x86, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Vivek Goyal
The code cleanup mentioned in Fixes tag changed the behavior of
kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
allocate free memory only when kbuf.mem is initialized as zero.
But in x86 kexec_file_load implementation there are a few places
the kbuf.mem is reused like below:
/* kbuf initialized, kbuf.mem = 0 */
...
kexec_add_buffer()
...
kexec_add_buffer()
The second kexec_add_buffer will reuse previous kbuf but not
reinitialize the kbuf.mem.
Thus kexec_file_load failed because the sanity check failed.
So explictily reset kbuf.mem to fix the issue.
Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
Signed-off-by: Dave Young <dyoung@redhat.com>
Cc: <stable@vger.kernel.org>
---
V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
arch/x86/kernel/crash.c | 1 +
arch/x86/kernel/kexec-bzimage64.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index f631a3f15587..6b7890c7889b 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
kbuf.memsz = kbuf.bufsz;
kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret) {
vfree((void *)image->arch.elf_headers);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 278cd07228dd..0d5efa34f359 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
kbuf.memsz = PAGE_ALIGN(header->init_size);
kbuf.buf_align = header->kernel_alignment;
kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret)
goto out_free_params;
@@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
kbuf.bufsz = kbuf.memsz = initrd_len;
kbuf.buf_align = PAGE_SIZE;
kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret)
goto out_free_params;
--
2.17.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2018-12-28 1:12 ` Dave Young
@ 2019-01-08 3:22 ` Dave Young
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 3:22 UTC (permalink / raw)
To: linux-kernel, kexec
Cc: Baoquan He, x86, AKASHI Takahiro, Ingo Molnar, Borislav Petkov,
Eric W. Biederman, Andrew Morton, Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
Ping, this is a regression issue, can we get this fixed?
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-08 3:22 ` Dave Young
0 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 3:22 UTC (permalink / raw)
To: linux-kernel, kexec
Cc: AKASHI Takahiro, Andrew Morton, Eric W. Biederman, Baoquan He,
x86, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
Ping, this is a regression issue, can we get this fixed?
Thanks
Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2018-12-28 1:12 ` Dave Young
@ 2019-01-08 5:24 ` Baoquan He
-1 siblings, 0 replies; 15+ messages in thread
From: Baoquan He @ 2019-01-08 5:24 UTC (permalink / raw)
To: Dave Young
Cc: x86, kexec, linux-kernel, AKASHI Takahiro, Ingo Molnar,
Borislav Petkov, Eric W. Biederman, Andrew Morton,
Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
Wondering why this place doesn't need the initialization assignment.
Isn't it to assign in all places before kexec_add_buffer() calling?
/* Add backup segment. */
if (image->arch.backup_src_sz) {
}
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
Same question for bzImage64_load(), there are three kexec_add_buffer()
calling, I only saw two initialization in this patch.
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-08 5:24 ` Baoquan He
0 siblings, 0 replies; 15+ messages in thread
From: Baoquan He @ 2019-01-08 5:24 UTC (permalink / raw)
To: Dave Young
Cc: linux-kernel, kexec, AKASHI Takahiro, Andrew Morton,
Eric W. Biederman, x86, Ingo Molnar, Borislav Petkov,
Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
Wondering why this place doesn't need the initialization assignment.
Isn't it to assign in all places before kexec_add_buffer() calling?
/* Add backup segment. */
if (image->arch.backup_src_sz) {
}
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
Same question for bzImage64_load(), there are three kexec_add_buffer()
calling, I only saw two initialization in this patch.
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2019-01-08 5:24 ` Baoquan He
@ 2019-01-08 8:46 ` Dave Young
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 8:46 UTC (permalink / raw)
To: Baoquan He
Cc: x86, kexec, linux-kernel, AKASHI Takahiro, Ingo Molnar,
Borislav Petkov, Eric W. Biederman, Andrew Morton,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 01:24pm, Baoquan He wrote:
> On 12/28/18 at 09:12am, Dave Young wrote:
> > The code cleanup mentioned in Fixes tag changed the behavior of
> > kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> > allocate free memory only when kbuf.mem is initialized as zero.
> >
> > But in x86 kexec_file_load implementation there are a few places
> > the kbuf.mem is reused like below:
> > /* kbuf initialized, kbuf.mem = 0 */
> > ...
> > kexec_add_buffer()
> > ...
> > kexec_add_buffer()
> >
> > The second kexec_add_buffer will reuse previous kbuf but not
> > reinitialize the kbuf.mem.
> >
> > Thus kexec_file_load failed because the sanity check failed.
> >
> > So explictily reset kbuf.mem to fix the issue.
> >
> > Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> > Signed-off-by: Dave Young <dyoung@redhat.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> > arch/x86/kernel/crash.c | 1 +
> > arch/x86/kernel/kexec-bzimage64.c | 2 ++
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > index f631a3f15587..6b7890c7889b 100644
> > --- a/arch/x86/kernel/crash.c
> > +++ b/arch/x86/kernel/crash.c
> > @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
> >
>
> Wondering why this place doesn't need the initialization assignment.
> Isn't it to assign in all places before kexec_add_buffer() calling?
C designated initializers will make sure to initialize it as zero.
We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
>
> /* Add backup segment. */
> if (image->arch.backup_src_sz) {
> }
>
> > kbuf.memsz = kbuf.bufsz;
> > kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret) {
> > vfree((void *)image->arch.elf_headers);
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > index 278cd07228dd..0d5efa34f359 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > kbuf.memsz = PAGE_ALIGN(header->init_size);
> > kbuf.buf_align = header->kernel_alignment;
> > kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
>
> Same question for bzImage64_load(), there are three kexec_add_buffer()
> calling, I only saw two initialization in this patch.
>
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret)
> > goto out_free_params;
> > @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > kbuf.bufsz = kbuf.memsz = initrd_len;
> > kbuf.buf_align = PAGE_SIZE;
> > kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret)
> > goto out_free_params;
> > --
> > 2.17.0
> >
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-08 8:46 ` Dave Young
0 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 8:46 UTC (permalink / raw)
To: Baoquan He
Cc: linux-kernel, kexec, AKASHI Takahiro, Andrew Morton,
Eric W. Biederman, x86, Ingo Molnar, Borislav Petkov,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 01:24pm, Baoquan He wrote:
> On 12/28/18 at 09:12am, Dave Young wrote:
> > The code cleanup mentioned in Fixes tag changed the behavior of
> > kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> > allocate free memory only when kbuf.mem is initialized as zero.
> >
> > But in x86 kexec_file_load implementation there are a few places
> > the kbuf.mem is reused like below:
> > /* kbuf initialized, kbuf.mem = 0 */
> > ...
> > kexec_add_buffer()
> > ...
> > kexec_add_buffer()
> >
> > The second kexec_add_buffer will reuse previous kbuf but not
> > reinitialize the kbuf.mem.
> >
> > Thus kexec_file_load failed because the sanity check failed.
> >
> > So explictily reset kbuf.mem to fix the issue.
> >
> > Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> > Signed-off-by: Dave Young <dyoung@redhat.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> > arch/x86/kernel/crash.c | 1 +
> > arch/x86/kernel/kexec-bzimage64.c | 2 ++
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > index f631a3f15587..6b7890c7889b 100644
> > --- a/arch/x86/kernel/crash.c
> > +++ b/arch/x86/kernel/crash.c
> > @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
> >
>
> Wondering why this place doesn't need the initialization assignment.
> Isn't it to assign in all places before kexec_add_buffer() calling?
C designated initializers will make sure to initialize it as zero.
We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
>
> /* Add backup segment. */
> if (image->arch.backup_src_sz) {
> }
>
> > kbuf.memsz = kbuf.bufsz;
> > kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret) {
> > vfree((void *)image->arch.elf_headers);
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > index 278cd07228dd..0d5efa34f359 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > kbuf.memsz = PAGE_ALIGN(header->init_size);
> > kbuf.buf_align = header->kernel_alignment;
> > kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
>
> Same question for bzImage64_load(), there are three kexec_add_buffer()
> calling, I only saw two initialization in this patch.
>
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret)
> > goto out_free_params;
> > @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > kbuf.bufsz = kbuf.memsz = initrd_len;
> > kbuf.buf_align = PAGE_SIZE;
> > kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> > + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret)
> > goto out_free_params;
> > --
> > 2.17.0
> >
Thanks
Dave
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2019-01-08 8:46 ` Dave Young
@ 2019-01-08 8:51 ` Baoquan He
-1 siblings, 0 replies; 15+ messages in thread
From: Baoquan He @ 2019-01-08 8:51 UTC (permalink / raw)
To: Dave Young
Cc: x86, kexec, linux-kernel, AKASHI Takahiro, Ingo Molnar,
Borislav Petkov, Eric W. Biederman, Andrew Morton,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 04:46pm, Dave Young wrote:
> > Wondering why this place doesn't need the initialization assignment.
> > Isn't it to assign in all places before kexec_add_buffer() calling?
>
> C designated initializers will make sure to initialize it as zero.
> We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
Got it, it works, thanks. People may need check code to find out
KEXEC_BUF_MEM_UNKNOWN is 0, then realize this fact.
Other than this, it looks good to me, ack it.
Acked-by: Baoquan He <bhe@redhat.com>
Thanks
Baoquan
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-08 8:51 ` Baoquan He
0 siblings, 0 replies; 15+ messages in thread
From: Baoquan He @ 2019-01-08 8:51 UTC (permalink / raw)
To: Dave Young
Cc: linux-kernel, kexec, AKASHI Takahiro, Andrew Morton,
Eric W. Biederman, x86, Ingo Molnar, Borislav Petkov,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 04:46pm, Dave Young wrote:
> > Wondering why this place doesn't need the initialization assignment.
> > Isn't it to assign in all places before kexec_add_buffer() calling?
>
> C designated initializers will make sure to initialize it as zero.
> We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
Got it, it works, thanks. People may need check code to find out
KEXEC_BUF_MEM_UNKNOWN is 0, then realize this fact.
Other than this, it looks good to me, ack it.
Acked-by: Baoquan He <bhe@redhat.com>
Thanks
Baoquan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2019-01-08 8:51 ` Baoquan He
@ 2019-01-08 9:11 ` Dave Young
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 9:11 UTC (permalink / raw)
To: Baoquan He
Cc: x86, kexec, linux-kernel, AKASHI Takahiro, Ingo Molnar,
Borislav Petkov, Eric W. Biederman, Andrew Morton,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 04:51pm, Baoquan He wrote:
> On 01/08/19 at 04:46pm, Dave Young wrote:
> > > Wondering why this place doesn't need the initialization assignment.
> > > Isn't it to assign in all places before kexec_add_buffer() calling?
> >
> > C designated initializers will make sure to initialize it as zero.
> > We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
>
> Got it, it works, thanks. People may need check code to find out
> KEXEC_BUF_MEM_UNKNOWN is 0, then realize this fact.
Agreed, it is not very clear now. It's better to improve it with some explict
initial value since we have the macro. But since this is a regression
I suggest to fix the bug first, I can send a patch later for the
improvement.
Thanks!
>
> Other than this, it looks good to me, ack it.
>
> Acked-by: Baoquan He <bhe@redhat.com>
>
> Thanks
> Baoquan
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-08 9:11 ` Dave Young
0 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-08 9:11 UTC (permalink / raw)
To: Baoquan He
Cc: linux-kernel, kexec, AKASHI Takahiro, Andrew Morton,
Eric W. Biederman, x86, Ingo Molnar, Borislav Petkov,
Thomas Gleixner, Vivek Goyal
On 01/08/19 at 04:51pm, Baoquan He wrote:
> On 01/08/19 at 04:46pm, Dave Young wrote:
> > > Wondering why this place doesn't need the initialization assignment.
> > > Isn't it to assign in all places before kexec_add_buffer() calling?
> >
> > C designated initializers will make sure to initialize it as zero.
> > We set KEXEC_BUF_MEM_UNKNOWN as 0 so it just works.
>
> Got it, it works, thanks. People may need check code to find out
> KEXEC_BUF_MEM_UNKNOWN is 0, then realize this fact.
Agreed, it is not very clear now. It's better to improve it with some explict
initial value since we have the macro. But since this is a regression
I suggest to fix the bug first, I can send a patch later for the
improvement.
Thanks!
>
> Other than this, it looks good to me, ack it.
>
> Acked-by: Baoquan He <bhe@redhat.com>
>
> Thanks
> Baoquan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
2018-12-28 1:12 ` Dave Young
@ 2019-01-15 5:15 ` Dave Young
-1 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-15 5:15 UTC (permalink / raw)
To: linux-kernel, kexec, Andrew Morton, Borislav Petkov
Cc: Baoquan He, x86, AKASHI Takahiro, Ingo Molnar, Eric W. Biederman,
Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
Andrew, Boris, can any of you take this patch? Without this fix we have a regression.
Thanks
Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2] x86/kexec: fix a kexec_file_load failure
@ 2019-01-15 5:15 ` Dave Young
0 siblings, 0 replies; 15+ messages in thread
From: Dave Young @ 2019-01-15 5:15 UTC (permalink / raw)
To: linux-kernel, kexec, Andrew Morton, Borislav Petkov
Cc: AKASHI Takahiro, Eric W. Biederman, Baoquan He, x86, Ingo Molnar,
Thomas Gleixner, Vivek Goyal
On 12/28/18 at 09:12am, Dave Young wrote:
> The code cleanup mentioned in Fixes tag changed the behavior of
> kexec_locate_mem_hole. The kexec_locate_mem_hole will try to
> allocate free memory only when kbuf.mem is initialized as zero.
>
> But in x86 kexec_file_load implementation there are a few places
> the kbuf.mem is reused like below:
> /* kbuf initialized, kbuf.mem = 0 */
> ...
> kexec_add_buffer()
> ...
> kexec_add_buffer()
>
> The second kexec_add_buffer will reuse previous kbuf but not
> reinitialize the kbuf.mem.
>
> Thus kexec_file_load failed because the sanity check failed.
>
> So explictily reset kbuf.mem to fix the issue.
>
> Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
> Signed-off-by: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2: use KEXEC_BUF_MEM_UNKNOWN in code.
> arch/x86/kernel/crash.c | 1 +
> arch/x86/kernel/kexec-bzimage64.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index f631a3f15587..6b7890c7889b 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -469,6 +469,7 @@ int crash_load_segments(struct kimage *image)
>
> kbuf.memsz = kbuf.bufsz;
> kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->arch.elf_headers);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 278cd07228dd..0d5efa34f359 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.memsz = PAGE_ALIGN(header->init_size);
> kbuf.buf_align = header->kernel_alignment;
> kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> @@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> kbuf.bufsz = kbuf.memsz = initrd_len;
> kbuf.buf_align = PAGE_SIZE;
> kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_params;
> --
> 2.17.0
>
Andrew, Boris, can any of you take this patch? Without this fix we have a regression.
Thanks
Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip:x86/urgent] x86/kexec: Fix a kexec_file_load() failure
2018-12-28 1:12 ` Dave Young
` (3 preceding siblings ...)
(?)
@ 2019-01-15 11:18 ` tip-bot for Dave Young
-1 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Dave Young @ 2019-01-15 11:18 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, x86, vgoyal, akpm, hpa, wang.yi59, schwidefsky, bhe, bp,
linux-kernel, mingo, ebiederm, mingo, yannik, dyoung, prudo,
takahiro.akashi
Commit-ID: 993a110319a4a60aadbd02f6defdebe048f7773b
Gitweb: https://git.kernel.org/tip/993a110319a4a60aadbd02f6defdebe048f7773b
Author: Dave Young <dyoung@redhat.com>
AuthorDate: Fri, 28 Dec 2018 09:12:47 +0800
Committer: Borislav Petkov <bp@suse.de>
CommitDate: Tue, 15 Jan 2019 12:12:50 +0100
x86/kexec: Fix a kexec_file_load() failure
Commit
b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
changed the behavior of kexec_locate_mem_hole(): it will try to allocate
free memory only when kbuf.mem is initialized to zero.
However, x86's kexec_file_load() implementation reuses a struct
kexec_buf allocated on the stack and its kbuf.mem member gets set by
each kexec_add_buffer() invocation.
The second kexec_add_buffer() will reuse the same kbuf but not
reinitialize kbuf.mem.
Therefore, explictily reset kbuf.mem each time in order for
kexec_locate_mem_hole() to locate a free memory region each time.
[ bp: massage commit message. ]
Fixes: b6664ba42f14 ("s390, kexec_file: drop arch_kexec_mem_walk()")
Signed-off-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Baoquan He <bhe@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Philipp Rudo <prudo@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Yannik Sembritzki <yannik@sembritzki.me>
Cc: Yi Wang <wang.yi59@zte.com.cn>
Cc: kexec@lists.infradead.org
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20181228011247.GA9999@dhcp-128-65.nay.redhat.com
---
arch/x86/kernel/crash.c | 1 +
arch/x86/kernel/kexec-bzimage64.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index c8b07d8ea5a2..17ffc869cab8 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -470,6 +470,7 @@ int crash_load_segments(struct kimage *image)
kbuf.memsz = kbuf.bufsz;
kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret) {
vfree((void *)image->arch.elf_headers);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 278cd07228dd..0d5efa34f359 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -434,6 +434,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
kbuf.memsz = PAGE_ALIGN(header->init_size);
kbuf.buf_align = header->kernel_alignment;
kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret)
goto out_free_params;
@@ -448,6 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
kbuf.bufsz = kbuf.memsz = initrd_len;
kbuf.buf_align = PAGE_SIZE;
kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
+ kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret)
goto out_free_params;
^ permalink raw reply related [flat|nested] 15+ messages in thread