From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0536F380FFA for ; Fri, 12 Jun 2026 18:25:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781288744; cv=none; b=XeB3GBzSJVfOOg7p3vky8M/1PKCBUAlVHNbLbWWlBMw5KjnwQQhD0vto9/u17O3UB+I1COHv0TzCW/wnxBaV+8TMqxSUiGIUNXNwbBIrFSjKHpBtt+eQNIAUe0kfaHJTZJUeq/m2vfmOY/NJX/u5xx/cdNyjrg5e8oDf+IXFv7c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781288744; c=relaxed/simple; bh=bs4phM0j5IY1/yRn9b4W8rearXuV//b/EhxOdiBnDSo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=q7LQey1iIL4WsMjEEZd9ypxTdDbVGRsuP9zTe5WBrJ27Y/wRAw5hY/TEugHeWwp89lzQesp58N7UiqY4jwaoGbg49vkzIGyZqUc1W5muWv1zVhYjJ9LR8tdXPe/VGYJ5/HYyC6Q+Hgr53Zk/R+AIesfcRjRAzI1nbSncJ8ldgs4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=O1kHwmd8; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="O1kHwmd8" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6F13E1F00A3A; Fri, 12 Jun 2026 18:25:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781288742; bh=WaUf1YVYcCYJGuNoId8/t188E3/+RbJhWQtJZWB54eg=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=O1kHwmd8/XDm/6WY3lxP5uVO2Ubpvz0SVTbloJqzlkCYWGNWY1CJGD3lR6J//O8cy S72r7IEqZr7sy5RpKMZZau6uMJ3vj3EsvKGpfV3fe61RqgZ/akE0jUdK4cOZDcCHAs CBVlmC00qKIABCpNQfA0VsYKijxQyRKbbpnhr+dOMyQtl4BGwJbxMkZUr22O2h4uRz ZwMu3xFrUFL9pz5aiRQs8xJh9BnUAZ0qfTfS54C8Wcjm2s0Zs+2TRPcMFRCICcIQr4 ruVGqLoMKIISrRbSk9TEak5sK+gt2myOcuGqk+/YUFAynG5hL28GB+c4S0I4FMXgBt GCGIyGc01KgkQ== Message-ID: <7a9d6acd-90db-4dfe-9fe5-55ba8adab577@kernel.org> Date: Fri, 12 Jun 2026 13:25:41 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Content-Language: en-US To: Lizhi Hou , ogabbay@kernel.org, quic_jhugo@quicinc.com, dri-devel@lists.freedesktop.org, karol.wachowski@linux.intel.com Cc: linux-kernel@vger.kernel.org, max.zhen@amd.com, sonal.santan@amd.com References: <20260611055150.3070216-1-lizhi.hou@amd.com> From: Mario Limonciello In-Reply-To: <20260611055150.3070216-1-lizhi.hou@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 6/11/26 00:51, Lizhi Hou wrote: > In amdxdna_remove(), all amdxdna_client structures are freed after > calling drm_dev_unplug(). However, drm_dev_unplug() does not force > existing file descriptors to be closed, so amdxdna_drm_close() may be > called after amdxdna_remove() has completed. > > As a result, accessing client->pid for debug output in > amdxdna_drm_close() can lead to a use-after-free, since the access is > not protected by drm_dev_enter(). > > Fix this by decoupling hardware teardown from client cleanup. > amdxdna_remove() only performs hardware-related cleanup, while > per-client resources are released from amdxdna_drm_close() when the > corresponding file is closed. > > Fixes: be462c97b7df ("accel/amdxdna: Add hardware context") > Signed-off-by: Lizhi Hou Reviewed-by: Mario Limonciello (AMD) > --- > drivers/accel/amdxdna/amdxdna_pci_drv.c | 23 ++++++++++------------- > drivers/accel/amdxdna/amdxdna_pci_drv.h | 1 + > 2 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c > index 65489bb3f2b0..470bf4fc744b 100644 > --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c > +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c > @@ -138,9 +138,11 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp) > xdna->dev_info->dev_heap_max_size); > mutex_init(&client->mm_lock); > > + mutex_lock(&xdna->client_lock); > mutex_lock(&xdna->dev_lock); > list_add_tail(&client->node, &xdna->client_list); > mutex_unlock(&xdna->dev_lock); > + mutex_unlock(&xdna->client_lock); > > filp->driver_priv = client; > client->filp = filp; > @@ -174,18 +176,14 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp) > { > struct amdxdna_client *client = filp->driver_priv; > struct amdxdna_dev *xdna = to_xdna_dev(ddev); > - int idx; > > XDNA_DBG(xdna, "closing pid %d", client->pid); > > - if (!drm_dev_enter(&xdna->ddev, &idx)) > - return; > - > + mutex_lock(&xdna->client_lock); > mutex_lock(&xdna->dev_lock); > amdxdna_client_cleanup(client); > mutex_unlock(&xdna->dev_lock); > - > - drm_dev_exit(idx); > + mutex_unlock(&xdna->client_lock); > } > > static int amdxdna_drm_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) > @@ -372,6 +370,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id) > return -ENODEV; > > drmm_mutex_init(ddev, &xdna->dev_lock); > + drmm_mutex_init(ddev, &xdna->client_lock); > init_rwsem(&xdna->notifier_lock); > INIT_LIST_HEAD(&xdna->client_list); > pci_set_drvdata(pdev, xdna); > @@ -442,18 +441,16 @@ static void amdxdna_remove(struct pci_dev *pdev) > drm_dev_unplug(&xdna->ddev); > amdxdna_sysfs_fini(xdna); > > + mutex_lock(&xdna->client_lock); > mutex_lock(&xdna->dev_lock); > - client = list_first_entry_or_null(&xdna->client_list, > - struct amdxdna_client, node); > - while (client) { > - amdxdna_client_cleanup(client); > - > - client = list_first_entry_or_null(&xdna->client_list, > - struct amdxdna_client, node); > + list_for_each_entry(client, &xdna->client_list, node) { > + amdxdna_hwctx_remove_all(client); > + amdxdna_sva_fini(client); > } > > xdna->dev_info->ops->fini(xdna); > mutex_unlock(&xdna->dev_lock); > + mutex_unlock(&xdna->client_lock); > > amdxdna_iommu_fini(xdna); > } > diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h > index 34271c14d359..a997d27a504d 100644 > --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h > +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h > @@ -120,6 +120,7 @@ struct amdxdna_dev { > > struct mutex dev_lock; /* per device lock */ > struct list_head client_list; > + struct mutex client_lock; /* client_list */ > struct amdxdna_fw_ver fw_ver; > struct rw_semaphore notifier_lock; /* for mmu notifier*/ > struct workqueue_struct *notifier_wq;