Re: [PATCH v8 3/6] kho: persist blob size in KHO FDT

Breno Leitao
2026-03-16T11:09:59Z

On Fri, Mar 13, 2026 at 09:21:50AM +0000, Pratyush Yadav wrote:
> On Mon, Mar 09 2026, Breno Leitao wrote:
>
> > kho_add_subtree() accepts a size parameter but only forwards it to
> > debugfs. The size is not persisted in the KHO FDT, so it is lost across
> > kexec. This makes it impossible for the incoming kernel to determine the
> > blob size without understanding the blob format.
> >
> > Store the blob size as a "blob-size" property in the KHO FDT alongside
> > the "preserved-data" physical address. This allows the receiving kernel
> > to recover the size for any blob regardless of format.
> >
> > Also extend kho_retrieve_subtree() with an optional size output
> > parameter so callers can learn the blob size without needing to
> > understand the blob format. Update all callers to pass NULL for the
> > new parameter.
> >
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> [...]
> > diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> > index 54fe59fe43acd..1f22705d5d246 100644
> > --- a/kernel/liveupdate/kexec_handover.c
> > +++ b/kernel/liveupdate/kexec_handover.c
> > @@ -768,6 +768,7 @@ int kho_add_subtree(const char *name, void *blob, size_t size)
> > {
> > phys_addr_t phys = virt_to_phys(blob);
> > void *root_fdt = kho_out.fdt;
> > + u64 size_u64 = size;
> > int err = -ENOMEM;
> > int off, fdt_err;
> >
> > @@ -784,11 +785,16 @@ int kho_add_subtree(const char *name, void *blob, size_t size)
> > goto out_pack;
> > }
> >
> > - err = fdt_setprop(root_fdt, off, KHO_FDT_SUB_TREE_PROP_NAME,
> > + err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_PROP_NAME,
> > &phys, sizeof(phys));
> > if (err < 0)
> > goto out_pack;
> >
> > + err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_SIZE_PROP_NAME,
> > + &size_u64, sizeof(size_u64));
> > + if (err < 0)
> > + goto out_pack;
> > +
>
> I noticed that the error handling here is a bit broken. We open the
> subnode for the subtree, but then if we fail to add the "preserved-data"
> property, we don't remove the subnode. So the next kernel gets an
> invalid FDT (per KHO ABI) and might as well refuse to parse it.
>
> Similarly here, the FDT might also be missing the size and then the next
> kernel might reject the FDT.
>
> Also, we directly return the FDT error code to the caller, which
> wouldn't make sense since it probably expects -errno.
>
> Not something this patchset has to fix, but I am pointing this out in
> case someone (possibly also future me) is interested in fixing this up.

That is a good point, do you mean a fix like the following?

commit 633d0cb01ed959676b60de8b1851dad1757d8fe5
Author: Breno Leitao <leitao@debian.org>
Date: Mon Mar 16 04:03:51 2026 -0700

kho: fix error handling in kho_add_subtree()

Fix two error handling issues in kho_add_subtree():

1. If fdt_setprop() fails after the subnode has been created, the
subnode is not removed. This leaves an incomplete node in the FDT
(missing "preserved-data" or "blob-size" properties), which violates
the KHO ABI and may cause the next kernel to reject the FDT.

2. The fdt_setprop() return value (an FDT error code) is stored
directly in err and returned to the caller, which expects -errno.

Fix both by storing fdt_setprop() results in fdt_err, jumping to a new
out_del_node label that removes the subnode on failure, and only setting
err = 0 on the success path.

Signed-off-by: Breno Leitao <leitao@debian.org>
Suggested-by: Pratyush Yadav <pratyush@kernel.org>

diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
index 62b1b8a9aa337..8d2d30119f6d4 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -787,19 +787,24 @@ int kho_add_subtree(const char *name, void *blob, size_t size)
goto out_pack;
}

- err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_PROP_NAME,
- &phys, sizeof(phys));
- if (err < 0)
- goto out_pack;
+ fdt_err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_PROP_NAME,
+ &phys, sizeof(phys));
+ if (fdt_err < 0)
+ goto out_del_node;

- err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_SIZE_PROP_NAME,
- &size_u64, sizeof(size_u64));
- if (err < 0)
- goto out_pack;
+ fdt_err = fdt_setprop(root_fdt, off, KHO_SUB_TREE_SIZE_PROP_NAME,
+ &size_u64, sizeof(size_u64));
+ if (fdt_err < 0)
+ goto out_del_node;

WARN_ON_ONCE(kho_debugfs_blob_add(&kho_out.dbg, name, blob,
size, false));

+ err = 0;
+ goto out_pack;
+
+out_del_node:
+ fdt_del_node(root_fdt, off);
out_pack:
fdt_pack(root_fdt);

Given this is not strictly related to this patchset, I am planning to
send this fix separately.

> > WARN_ON_ONCE(kho_debugfs_blob_add(&kho_out.dbg, name, blob,
> > size, false));
> >
> > @@ -817,7 +823,7 @@ void kho_remove_subtree(void *blob)
> > const u64 *val;
> > int len;
> >
> > - val = fdt_getprop(root_fdt, off, KHO_FDT_SUB_TREE_PROP_NAME, &len);
> > + val = fdt_getprop(root_fdt, off, KHO_SUB_TREE_PROP_NAME, &len);
> > if (!val || len != sizeof(phys_addr_t))
> > continue;
> >
> > @@ -1314,13 +1320,14 @@ EXPORT_SYMBOL_GPL(is_kho_boot);
> > * kho_retrieve_subtree - retrieve a preserved sub blob by its name.
> > * @name: the name of the sub blob passed to kho_add_subtree().
> > * @phys: if found, the physical address of the sub blob is stored in @phys.
> > + * @size: if not NULL and found, the size of the sub blob is stored in @size.
> > *
> > * Retrieve a preserved sub blob named @name and store its physical
> > - * address in @phys.
> > + * address in @phys and optionally its size in @size.
> > *
> > * Return: 0 on success, error code on failure
> > */
> > -int kho_retrieve_subtree(const char *name, phys_addr_t *phys)
> > +int kho_retrieve_subtree(const char *name, phys_addr_t *phys, size_t *size)
> > {
> > const void *fdt = kho_get_fdt();
> > const u64 *val;
> > @@ -1336,12 +1343,21 @@ int kho_retrieve_subtree(const char *name, phys_addr_t *phys)
> > if (offset < 0)
> > return -ENOENT;
> >
> > - val = fdt_getprop(fdt, offset, KHO_FDT_SUB_TREE_PROP_NAME, &len);
> > + val = fdt_getprop(fdt, offset, KHO_SUB_TREE_PROP_NAME, &len);
> > if (!val || len != sizeof(*val))
> > return -EINVAL;
> >
> > *phys = (phys_addr_t)*val;
> >
> > + if (size) {
> > + val = fdt_getprop(fdt, offset, KHO_SUB_TREE_SIZE_PROP_NAME,
> > + &len);
> > + if (val && len == sizeof(*val))
> > + *size = (size_t)*val;
> > + else
> > + *size = 0;
>
> If the size property is invalid, is it a good idea to ignore it? Should
> we instead consider the subnode to be broken and reject it entirely with
> an error message? Because if a caller expects a blob of 16 bytes but
> gets one with 0 bytes, it will likely error out anyway.

Ack, let me update this, then.

Thanks for the review,
--breno