Re: [PATCH v3] node: put_device after failing to device_register

[Johan Hovold <johan@kernel.org>]

On Wed, Jun 15, 2022 at 11:17:38PM +0800, Zhi Song wrote:
> device_register() is used to register a device with the system.
> We need to call put_device() to give up the reference initialized
> in device_register() when it returns an error and this will clean
> up correctly.
> 
> Fixes: 08d9dbe72b1f ("node: Link memory nodes to their compute nodes")
> Signed-off-by: Zhi Song <zhi.song@bytedance.com>
> ---
> V1 -> V2: Fix up the changelog text correct.
> V2 -> V3: Add a fixes tag line specifying the commit where this bug was
> introduced.
> ---
>  drivers/base/node.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 0ac6376ef7a1..88a3337c546e 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -154,6 +154,7 @@ static struct node_access_nodes *node_init_node_access(struct node *node,
>  	list_add_tail(&access_node->list_node, &node->access_list);
>  	return access_node;
>  free_name:
> +	put_device(dev);
>  	kfree_const(dev->kobj.name);

That's a pretty obvious use-after-free you just added here. You can't
access dev after you've just freed it.

The name is freed along with the rest of the struct device so you need
to remove the second explicit free. And you should rename the label too.

>  free:
>  	kfree(access_node);

But here's another use after free... The put_device() call you added
will have freed access_node by calling node_access_release().

Johan