Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 20 Jun 2022 15:55:56 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Zhi Song <zhi.song@bytedance.com>
Cc:     gregkh@linuxfoundation.org, rafael@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] node: put_device after failing to device_register
Message-ID: <YrB8bIJj3fvvsCHg@hovoldconsulting.com>
References: <20220615151738.1766206-1-zhi.song@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220615151738.1766206-1-zhi.song@bytedance.com>
Precedence: bulk

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