| Message ID | 20210421163610.23775-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2,1/2] powerpc: Free fdt on error in elf64_load() | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/merge (d20f726744a0312b4b6613333bda7da9bc52fb75) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/next (39352430aaa05fbe4ba710231c70b334513078f2) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linus/master (1fe5501ba1abf2b7e78295df73675423bd6899a0) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/fixes (791f9e36599d94af5a76d3f74d04e16326761aae) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linux-next (1fe5501ba1abf2b7e78295df73675423bd6899a0) |
On 4/21/21 9:36 AM, Lakshmi Ramasubramanian wrote: Hi Dan, > There are a few "goto out;" statements before the local variable "fdt" > is initialized through the call to of_kexec_alloc_and_setup_fdt() in > elf64_load(). This will result in an uninitialized "fdt" being passed > to kvfree() in this function if there is an error before the call to > of_kexec_alloc_and_setup_fdt(). > > If there is any error after fdt is allocated, but before it is > saved in the arch specific kimage struct, free the fdt. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > --- > arch/powerpc/kexec/elf_64.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > Please review this patch and Patch 2/2. thanks, -lakshmi > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c > index 5a569bb51349..02662e72c53d 100644 > --- a/arch/powerpc/kexec/elf_64.c > +++ b/arch/powerpc/kexec/elf_64.c > @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, > ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr, > initrd_len, cmdline); > if (ret) > - goto out; > + goto out_free_fdt; > > fdt_pack(fdt); > > @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, > kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; > ret = kexec_add_buffer(&kbuf); > if (ret) > - goto out; > + goto out_free_fdt; > > /* FDT will be freed in arch_kimage_file_post_load_cleanup */ > image->arch.fdt = fdt; > @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, > if (ret) > pr_err("Error setting up the purgatory.\n"); > > + goto out; > + > +out_free_fdt: > + kvfree(fdt); > out: > kfree(modified_cmdline); > kexec_free_elf_info(&elf_info); > > - /* > - * Once FDT buffer has been successfully passed to kexec_add_buffer(), > - * the FDT buffer address is saved in image->arch.fdt. In that case, > - * the memory cannot be freed here in case of any other error. > - */ > - if (ret && !image->arch.fdt) > - kvfree(fdt); > - > return ret ? ERR_PTR(ret) : NULL; > } > >
On Wed, 21 Apr 2021 09:36:09 -0700, Lakshmi Ramasubramanian wrote: > There are a few "goto out;" statements before the local variable "fdt" > is initialized through the call to of_kexec_alloc_and_setup_fdt() in > elf64_load(). This will result in an uninitialized "fdt" being passed > to kvfree() in this function if there is an error before the call to > of_kexec_alloc_and_setup_fdt(). > > If there is any error after fdt is allocated, but before it is > saved in the arch specific kimage struct, free the fdt. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > --- > arch/powerpc/kexec/elf_64.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > Applied, thanks!
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c index 5a569bb51349..02662e72c53d 100644 --- a/arch/powerpc/kexec/elf_64.c +++ b/arch/powerpc/kexec/elf_64.c @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr, initrd_len, cmdline); if (ret) - goto out; + goto out_free_fdt; fdt_pack(fdt); @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; ret = kexec_add_buffer(&kbuf); if (ret) - goto out; + goto out_free_fdt; /* FDT will be freed in arch_kimage_file_post_load_cleanup */ image->arch.fdt = fdt; @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, if (ret) pr_err("Error setting up the purgatory.\n"); + goto out; + +out_free_fdt: + kvfree(fdt); out: kfree(modified_cmdline); kexec_free_elf_info(&elf_info); - /* - * Once FDT buffer has been successfully passed to kexec_add_buffer(), - * the FDT buffer address is saved in image->arch.fdt. In that case, - * the memory cannot be freed here in case of any other error. - */ - if (ret && !image->arch.fdt) - kvfree(fdt); - return ret ? ERR_PTR(ret) : NULL; }