On Fri, Oct 24, 2025 at 11:52 AM Pratyush Yadav pratyush@kernel.org wrote:
On Fri, Oct 24 2025, Pratyush Yadav wrote:
On Fri, Oct 24 2025, Pasha Tatashin wrote:
-int kho_add_subtree(struct kho_serialization *ser, const char *name, void *fdt) +int kho_add_subtree(const char *name, void *fdt) {
int err = 0;u64 phys = (u64)virt_to_phys(fdt);void *root = page_to_virt(ser->fdt);
struct kho_sub_fdt *sub_fdt;int err;
err |= fdt_begin_node(root, name);err |= fdt_property(root, PROP_SUB_FDT, &phys, sizeof(phys));err |= fdt_end_node(root);
sub_fdt = kmalloc(sizeof(*sub_fdt), GFP_KERNEL);if (!sub_fdt)return -ENOMEM;
if (err)return err;
INIT_LIST_HEAD(&sub_fdt->l);sub_fdt->name = name;sub_fdt->fdt = fdt;
return kho_debugfs_fdt_add(&kho_out.dbg, name, fdt, false);
mutex_lock(&kho_out.fdts_lock);list_add_tail(&sub_fdt->l, &kho_out.sub_fdts);err = kho_debugfs_fdt_add(&kho_out.dbg, name, fdt, false);I think you should remove sub_fdt from the list and kfree() it on error here. Otherwise we signal an error to the caller and they might free sub_fdt->fdt, which will later result in a use-after-free at __kho_finalize().
I think, it is better to simply do: WARN_ON_ONCE(kho_debugfs_fdt_add(...)); Now debugfs is optional, and there is no reason to return an error to a caller if kho_debugfs_fdt_add() fails
Yeah, that works too.
On a second thought, maybe pr_warn() instead of WARN_ON()? This isn't an assertion since the debugfs creation can fail for many reasons. It isn't expected to always succeed. So a full WARN_ON() splat seems overkill.
I sent it with WARN_ON_ONCE(), I can change it to pr_warn_once() if there is another revision, otherwise we can just send a separate patch to make the change it is not that important.
Pasha
[...]
-- Regards, Pratyush Yadav