Re: [PATCH v6 08/10] liveupdate: kho: warn and fail on metadata or preserved memory in scratch area

...

+config KEXEC_HANDOVER_DEBUG

• bool "Enable Kexec Handover debug checks"
  

• depends on KEXEC_HANDOVER_DEBUGFS
  

• help
  

•   This option enables extra sanity checks for the Kexec Handover
  

•   subsystem. Since, KHO performance is crucial in live update
  

•   scenarios and the extra code might be adding overhead it is
  

•   only optionally enabled.
  

And empty line here would be nice.

Done.

...

endmenu diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c index c87d00c40c82..ebfc31814d16 100644 --- a/kernel/liveupdate/kexec_handover.c +++ b/kernel/liveupdate/kexec_handover.c @@ -8,6 +8,7 @@

#define pr_fmt(fmt) "KHO: " fmt

+#include <linux/cleanup.h> #include <linux/cma.h> #include <linux/count_zeros.h> #include <linux/kexec.h> @@ -131,26 +132,26 @@ static struct kho_out kho_out = {

static void *xa_load_or_alloc(struct xarray *xa, unsigned long index, size_t sz) {

• void *elm, *res;
  

• void *res = xa_load(xa, index);
  
  

• elm = xa_load(xa, index);
  

• if (elm)
  

•         return elm;
  

• if (res)
  

•         return res;
  

• void *elm __free(kfree) = kzalloc(sz, GFP_KERNEL);
  
  

• elm = kzalloc(sz, GFP_KERNEL);
  if (!elm)
          return ERR_PTR(-ENOMEM);
  
  

• if (WARN_ON(kho_scratch_overlap(virt_to_phys(elm), sz)))
  

I'd move the WARN_ON into kho_scratch_overlap().

I do not like O(N^2), let's keep it outside, even though it is a debug kernel, there is no need to make it unnecessary slow, when we are saving just two lines of code.

...

•         return ERR_PTR(-EINVAL);
  

• res = xa_cmpxchg(xa, index, NULL, elm, GFP_KERNEL);
  if (xa_is_err(res))
  

•         res = ERR_PTR(xa_err(res));
  

• if (res) {
  

•         kfree(elm);
  

•         return ERR_PTR(xa_err(res));
  

• else if (res)
          return res;
  

• }
  
  

• return elm;
  

• return no_free_ptr(elm);
  

}

...

...

@@ -379,14 +384,17 @@ static int kho_mem_serialize(struct kho_out *kho_out) struct khoser_mem_chunk *chunk = NULL; struct kho_mem_phys *physxa; unsigned long order;

• int ret = -ENOMEM;
  

Nit: s/ret/err/

Done.

...

  xa_for_each(&kho_out->track.orders, order, physxa) {
          struct kho_mem_phys_bits *bits;
          unsigned long phys;

diff --git a/kernel/liveupdate/kexec_handover_debug.c b/kernel/liveupdate/kexec_handover_debug.c new file mode 100644 index 000000000000..7986dcc63047 --- /dev/null +++ b/kernel/liveupdate/kexec_handover_debug.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*

• • kexec_handover_debug.c - kexec handover optional debug functionality
• • Copyright (C) 2025 Google LLC, Pasha Tatashin pasha.tatashin@soleen.com
• */

+#define pr_fmt(fmt) "KHO: " fmt

+#include "kexec_handover_internal.h"

+bool kho_scratch_overlap(phys_addr_t phys, size_t size) +{

• phys_addr_t scratch_start, scratch_end;
  

• unsigned int i;
  

• for (i = 0; i < kho_scratch_cnt; i++) {
  

•         scratch_start = kho_scratch[i].addr;
  

•         scratch_end = kho_scratch[i].addr + kho_scratch[i].size - 1;
  

I agree with Pratyush that

            scratch_end = kho_scratch[i].addr + kho_scratch[i].size;

            if (phys < scratch_end ...

is clearer.

Done.

Thanks, Pasha