Re: [PATCH v6 1/4] kho: add size parameter to kho_add_subtree()

[Pratyush Yadav <pratyush@kernel.org>]

Hi Breno,

On Tue, Jan 27 2026, Breno Leitao wrote:

> kho_add_subtree() assumes the fdt argument is always an FDT and calls
> fdt_totalsize() on it in the debugfs code path. This assumption will
> break if a caller passes arbitrary data instead of an FDT.
>
> When CONFIG_KEXEC_HANDOVER_DEBUGFS is enabled, kho_debugfs_fdt_add()
> calls __kho_debugfs_fdt_add(), which executes:
>
>     f->wrapper.size = fdt_totalsize(fdt);
>
> Fix this by adding an explicit size parameter to kho_add_subtree() so
> callers specify the blob size. This allows subtrees to contain
> arbitrary data formats, not just FDTs. Update all callers:
>
>   - memblock.c: use fdt_totalsize(fdt)
>   - luo_core.c: use fdt_totalsize(fdt_out)
>   - test_kho.c: use fdt_totalsize()
>   - kexec_handover.c (root fdt): use fdt_totalsize(kho_out.fdt)
>
> Also update kho_in_debugfs_init() to compute sizes using fdt_totalsize()
> for the root and sub-FDTs it processes, since these are known to be
> actual FDT blobs.

No, this doesn't seem right. The "fdt" field that kho_in_debugfs_init()
uses is the same "fdt" field where we put our non-FDT blobs. So I don't
see how these can be known to be actual FDTs. All this happened to work
because so far all users were FDT, but I bet it will break if you use
your blob here. Perhaps give it a try and see if I am understanding this
correctly?

To be honest, I didn't think of this when I suggested you use the size
parameter. We have lost the size information, and it is hard for
kho_in_debugfs_init() to find out since it has no idea what the
underlying format is.

One option is to have it in the KHO FDT, but I am not sure that is a
good idea. Adding to ABI for debug feature sounds odd (not that I am
completely against it, it just feels off).

Another would be to give users a hook to populate the blobs when they
call kho_retrieve_subtree(), so they can figure out how large the blob
needs to be. This has another benefit: once we move away from FDT, it
makes little sense to dump the blob since userspace won't have a way to
parse it. Even with FDT, userspace still can't parse everything. For
example, say the FDT has a reference to a struct kho_vmalloc. You'd get
a pointer to the head of the list, but you would have no way of knowing
what is inside the vmalloc buffer. This has the downside of not being
able to show anything if the subsystem never calls
kho_retrieve_subtree().

Unfortunately I don't have much time this week to dive deeper into this.
These are only things off the top of my head and I haven't thought too
deeply, so please don't take them as strong suggestions. It would be
great if you can think a bit more about the problem and come up with a
recommendation?

I will try to get back to this series in the next 1-2 weeks and
hopefully find some way to make progress.

I skimmed the rest of the patches and they all LGTM at a high level.

[...]

-- 
Regards,
Pratyush Yadav