Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] kho_radix_destroy_tree() leaves tree->root dangling, causing 
subsequent re-initializations to erroneously succeed and lead to Use-After-Free.
--

commit d207a159c7c112b64706fe2fb64399a65932fdb6
Author: Pratyush Yadav (Google) <[email protected]>

kho: add kho_radix_init_tree()

Move the initialization logic of the radix tree into
kho_radix_init_tree() instead of having users open-code it. Makes the
boundaries cleaner and reduces code duplication when a new user of the
radix tree will be added in a future commit.

[ ... ]

> diff --git a/kernel/liveupdate/kexec_handover.c 
> b/kernel/liveupdate/kexec_handover.c
> index 3f3ea71baa1ad..b2d1572808ebf 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -305,6 +305,34 @@ static void __kho_radix_destroy_tree(struct 
> kho_radix_node *root,
>       kho_radix_free_node(root);
>  }
>  
> +/**
> + * kho_radix_init_tree - initialize the radix tree.
> + * @tree:   the tree to initialize.
> + * @root:   root table of the radix tree.
> + *
> + * Initialize the radix tree with the given root node. If root is %NULL, an
> + * empty root table is allocated. If root is not %NULL, it is the caller's
> + * responsibility to make sure the root is valid and in the correct format.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +int kho_radix_init_tree(struct kho_radix_tree *tree, struct kho_radix_node 
> *root)
> +{
> +     /* Already initialized. */
> +     if (tree->root)
> +             return 0;

[Severity: High]
Can this cause a use-after-free if a tree is re-initialized?

Looking at kho_radix_destroy_tree(), it frees the tree nodes but does not set
tree->root to NULL:

kernel/liveupdate/kexec_handover.c:kho_radix_destroy_tree() {
        if (!tree->root)
                return;

        __kho_radix_destroy_tree(tree->root, KHO_TREE_MAX_DEPTH - 1);
}

If a tree is destroyed and then kho_radix_init_tree() is called on it again,
wouldn't this check see the dangling tree->root pointer and incorrectly return
success, leading to a use-after-free on subsequent tree operations?

> +
> +     if (!root)
> +             root = kho_radix_alloc_node();
> +     if (!root)
> +             return -ENOMEM;
> +
> +     tree->root = root;
> +     mutex_init(&tree->lock);
> +     return 0;
> +}
> +EXPORT_SYMBOL_GPL(kho_radix_init_tree);

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=8

Reply via email to