linux-kernel - Re: [PATCH v3] node: put_device after failing to device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YrB8bIJj3fvvsCHg@hovoldconsulting.com>
Date:   Mon, 20 Jun 2022 15:55:56 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Zhi Song <zhi.song@...edance.com>
Cc:     gregkh@...uxfoundation.org, rafael@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] node: put_device after failing to device_register

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@...edance.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