linux-kernel - Re: [PATCH] kho: cleanup error handling in kho

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+CK2bB-JZEFp57rCYK8wGKoGP8U1NX_nLd=RRu3ozuVn339UQ@mail.gmail.com>
Date: Thu, 22 Jan 2026 13:12:18 -0500
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Mike Rapoport <rppt@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Alexander Graf <graf@...zon.com>, 
	Pratyush Yadav <ptyadav@...zon.de>, kexec@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org
Subject: Re: [PATCH] kho: cleanup error handling in kho_populate()

On Thu, Jan 22, 2026 at 7:18 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> From: "Mike Rapoport (Microsoft)" <rppt@...nel.org>
>
> * use dedicated labels for error handling instead of checking if a pointer
>   is not null to decide if it should be unmapped
> * drop assignment of values to err that are only used to print a numeric
>   error code, there are pr_warn()s for each failure already so printing a
>   numeric error code in the next line does not add anything useful
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> ---
>  kernel/liveupdate/kexec_handover.c | 39 +++++++++++++-----------------
>  1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index d4482b6e3cae..e248ce2e04b7 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1441,46 +1441,40 @@ void __init kho_memory_init(void)
>  void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>                          phys_addr_t scratch_phys, u64 scratch_len)
>  {
> +       unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
>         struct kho_scratch *scratch = NULL;
>         phys_addr_t mem_map_phys;
>         void *fdt = NULL;
> -       int err = 0;
> -       unsigned int scratch_cnt = scratch_len / sizeof(*kho_scratch);
> +       int err;
>
>         /* Validate the input FDT */
>         fdt = early_memremap(fdt_phys, fdt_len);
>         if (!fdt) {
>                 pr_warn("setup: failed to memremap FDT (0x%llx)\n", fdt_phys);
> -               err = -EFAULT;
> -               goto out;
> +               goto err_report;
>         }
>         err = fdt_check_header(fdt);
>         if (err) {
>                 pr_warn("setup: handover FDT (0x%llx) is invalid: %d\n",
>                         fdt_phys, err);
> -               err = -EINVAL;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>         err = fdt_node_check_compatible(fdt, 0, KHO_FDT_COMPATIBLE);
>         if (err) {
>                 pr_warn("setup: handover FDT (0x%llx) is incompatible with '%s': %d\n",
>                         fdt_phys, KHO_FDT_COMPATIBLE, err);
> -               err = -EINVAL;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>
>         mem_map_phys = kho_get_mem_map_phys(fdt);
> -       if (!mem_map_phys) {
> -               err = -ENOENT;
> -               goto out;
> -       }
> +       if (!mem_map_phys)
> +               goto err_unmap_fdt;
>
>         scratch = early_memremap(scratch_phys, scratch_len);
>         if (!scratch) {
>                 pr_warn("setup: failed to memremap scratch (phys=0x%llx, len=%lld)\n",
>                         scratch_phys, scratch_len);
> -               err = -EFAULT;
> -               goto out;
> +               goto err_unmap_fdt;
>         }
>
>         /*
> @@ -1497,7 +1491,7 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>                 if (WARN_ON(err)) {
>                         pr_warn("failed to mark the scratch region 0x%pa+0x%pa: %pe",
>                                 &area->addr, &size, ERR_PTR(err));
> -                       goto out;
> +                       goto err_unmap_scratch;
>                 }
>                 pr_debug("Marked 0x%pa+0x%pa as scratch", &area->addr, &size);
>         }
> @@ -1519,13 +1513,14 @@ void __init kho_populate(phys_addr_t fdt_phys, u64 fdt_len,
>         kho_scratch_cnt = scratch_cnt;
>         pr_info("found kexec handover data.\n");
>
> -out:
> -       if (fdt)
> -               early_memunmap(fdt, fdt_len);
> -       if (scratch)
> -               early_memunmap(scratch, scratch_len);
> -       if (err)
> -               pr_warn("disabling KHO revival: %d\n", err);
> +       return;
> +
> +err_unmap_scratch:
> +       early_memunmap(scratch, scratch_len);
> +err_unmap_fdt:
> +       early_memunmap(fdt, fdt_len);
> +err_report:
> +       pr_warn("disabling KHO revival\n");

Reviewed-by: Pasha Tatashin <pasha.tatashin@...een.com>

Nice clean-up, thanks.

Pasha