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.
>
>>
>> > + mutex_unlock(&kho_out.fdts_lock);
>> > +
>> > + return err;
>> > }
>> > EXPORT_SYMBOL_GPL(kho_add_subtree);
>> >
>> > -int register_kho_notifier(struct notifier_block *nb)
>> > +void kho_remove_subtree(void *fdt)
>> > {
>> > - return blocking_notifier_chain_register(&kho_out.chain_head, nb);
>> > -}
>> > -EXPORT_SYMBOL_GPL(register_kho_notifier);
>> > + struct kho_sub_fdt *sub_fdt;
>> > +
>> > + mutex_lock(&kho_out.fdts_lock);
>> > + list_for_each_entry(sub_fdt, &kho_out.sub_fdts, l) {
>>
>> list_for_each_entry_safe() here since we delete.
>
> Not needed, we are breaking from the iterator when deleting.
Makes sense. Didn't think of that.
[...]
--
Regards,
Pratyush Yadav
