Memory safety - NULL pointer dereference in DMA fence handling

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c index e9fbd8c143641a..54486960cbf5e6 100644 --- a/drivers/accel/amdxdna/aie2_ctx.c +++ b/drivers/accel/amdxdna/aie2_ctx.c @@ -59,6 +59,18 @@ static bool aie2_tdr_detect(struct amdxdna_dev *xdna) return false; } +static void aie2_cmd_release(struct kref *ref) +{ + struct amdxdna_drv_cmd *drv_cmd = container_of(ref, struct amdxdna_drv_cmd, refcnt); + + kfree(drv_cmd); +} + +static void aie2_cmd_put(struct amdxdna_drv_cmd *drv_cmd) +{ + kref_put(&drv_cmd->refcnt, aie2_cmd_release); +} + static void aie2_job_release(struct kref *ref) { struct amdxdna_sched_job *job; @@ -70,6 +82,8 @@ static void aie2_job_release(struct kref *ref) wake_up(&job->hwctx->priv->job_free_wq); if (job->out_fence) dma_fence_put(job->out_fence); + if (job->drv_cmd) + aie2_cmd_put(job->drv_cmd); kfree(job->aie2_job_health); kfree(job); } @@ -901,7 +915,7 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl, { struct amdxdna_client *client = hwctx->client; struct amdxdna_dev *xdna = client->xdna; - struct amdxdna_drv_cmd cmd = { 0 }; + struct amdxdna_drv_cmd *cmd; struct amdxdna_gem_obj *abo; u64 seq; int ret; @@ -912,32 +926,39 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl, return -EINVAL; } + cmd = kzalloc_obj(*cmd); + if (!cmd) { + ret = -ENOMEM; + goto put_obj; + } + kref_init(&cmd->refcnt); + if (attach) { if (abo->assigned_hwctx != AMDXDNA_INVALID_CTX_HANDLE) { ret = -EBUSY; - goto put_obj; + goto put_cmd; } - cmd.opcode = ATTACH_DEBUG_BO; + cmd->opcode = ATTACH_DEBUG_BO; } else { if (abo->assigned_hwctx != hwctx->id) { ret = -EINVAL; - goto put_obj; + goto put_cmd; } - cmd.opcode = DETACH_DEBUG_BO; + cmd->opcode = DETACH_DEBUG_BO; } - ret = amdxdna_cmd_submit(client, &cmd, AMDXDNA_INVALID_BO_HANDLE, + ret = amdxdna_cmd_submit(client, cmd, AMDXDNA_INVALID_BO_HANDLE, &bo_hdl, 1, hwctx->id, &seq); if (ret) { XDNA_ERR(xdna, "Submit command failed"); - goto put_obj; + goto put_cmd; } aie2_cmd_wait(hwctx, seq); - if (cmd.result) { - XDNA_ERR(xdna, "Response failure 0x%x", cmd.result); + if (cmd->result) { + XDNA_ERR(xdna, "Response failure 0x%x", cmd->result); ret = -EINVAL; - goto put_obj; + goto put_cmd; } if (attach) @@ -947,6 +968,8 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl, XDNA_DBG(xdna, "Config debug BO %d to %s", bo_hdl, hwctx->name); +put_cmd: + aie2_cmd_put(cmd); put_obj: amdxdna_gem_put_obj(abo); return ret; @@ -974,25 +997,32 @@ int aie2_hwctx_sync_debug_bo(struct amdxdna_hwctx *hwctx, u32 debug_bo_hdl) { struct amdxdna_client *client = hwctx->client; struct amdxdna_dev *xdna = client->xdna; - struct amdxdna_drv_cmd cmd = { 0 }; + struct amdxdna_drv_cmd *cmd; u64 seq; int ret; - cmd.opcode = SYNC_DEBUG_BO; - ret = amdxdna_cmd_submit(client, &cmd, AMDXDNA_INVALID_BO_HANDLE, + cmd = kzalloc_obj(*cmd); + if (!cmd) + return -ENOMEM; + kref_init(&cmd->refcnt); + + cmd->opcode = SYNC_DEBUG_BO; + ret = amdxdna_cmd_submit(client, cmd, AMDXDNA_INVALID_BO_HANDLE, &debug_bo_hdl, 1, hwctx->id, &seq); if (ret) { XDNA_ERR(xdna, "Submit command failed"); - return ret; + goto put_cmd; } aie2_cmd_wait(hwctx, seq); - if (cmd.result) { - XDNA_ERR(xdna, "Response failure 0x%x", cmd.result); - return -EINVAL; + if (cmd->result) { + XDNA_ERR(xdna, "Response failure 0x%x", cmd->result); + ret = -EINVAL; } - return 0; +put_cmd: + aie2_cmd_put(cmd); + return ret; } static int aie2_populate_range(struct amdxdna_gem_obj *abo) @@ -1142,6 +1172,8 @@ int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, dma_resv_add_fence(job->bos[i]->resv, job->out_fence, DMA_RESV_USAGE_WRITE); job->seq = hwctx->priv->seq++; kref_get(&job->refcnt); + if (job->drv_cmd) + kref_get(&job->drv_cmd->refcnt); drm_sched_entity_push_job(&job->base); *seq = job->seq; diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h index aaae1643046619..b6bef3af7dab46 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.h +++ b/drivers/accel/amdxdna/amdxdna_ctx.h @@ -132,6 +132,7 @@ enum amdxdna_job_opcode { struct amdxdna_drv_cmd { enum amdxdna_job_opcode opcode; u32 result; + struct kref refcnt; }; struct app_health_report; diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c index eff00131d0f809..4f245b969eef1f 100644 --- a/drivers/accel/amdxdna/amdxdna_iommu.c +++ b/drivers/accel/amdxdna/amdxdna_iommu.c @@ -4,6 +4,7 @@ */ #include <drm/amdxdna_accel.h> +#include <drm/drm_managed.h> #include <linux/iommu.h> #include <linux/iova.h> @@ -153,10 +154,30 @@ void amdxdna_iommu_free(struct amdxdna_dev *xdna, size_t size, free_pages((unsigned long)cpu_addr, get_order(size)); } +static void amdxdna_cleanup_force_iova(struct drm_device *dev, void *res) +{ + struct amdxdna_dev *xdna = to_xdna_dev(dev); + + if (xdna->domain) { + iommu_detach_group(xdna->domain, xdna->group); + put_iova_domain(&xdna->iovad); + iova_cache_put(); + iommu_domain_free(xdna->domain); + } + + iommu_group_put(xdna->group); +} + +void amdxdna_iommu_fini(struct amdxdna_dev *xdna) +{ + if (xdna->group && !xdna->domain) + iommu_group_put(xdna->group); +} + int amdxdna_iommu_init(struct amdxdna_dev *xdna) { unsigned long order; - int ret; + int ret = 0; xdna->group = iommu_group_get(xdna->ddev.dev); if (!xdna->group || !force_iova) @@ -182,8 +203,14 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna) if (ret) goto put_iova; + ret = drmm_add_action(&xdna->ddev, amdxdna_cleanup_force_iova, NULL); + if (ret) + goto detach_group; + return 0; +detach_group: + iommu_detach_group(xdna->domain, xdna->group); put_iova: put_iova_domain(&xdna->iovad); iova_cache_put(); @@ -191,20 +218,8 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna) iommu_domain_free(xdna->domain); put_group: iommu_group_put(xdna->group); + xdna->group = NULL; xdna->domain = NULL; return ret; } - -void amdxdna_iommu_fini(struct amdxdna_dev *xdna) -{ - if (xdna->domain) { - iommu_detach_group(xdna->domain, xdna->group); - put_iova_domain(&xdna->iovad); - iova_cache_put(); - iommu_domain_free(xdna->domain); - } - - if (xdna->group) - iommu_group_put(xdna->group); -} diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c index 65489bb3f2b074..e94d8290a80738 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) @@ -371,6 +369,10 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (!xdna->dev_info) return -ENODEV; + ret = drmm_mutex_init(ddev, &xdna->client_lock); + if (ret) + return ret; + drmm_mutex_init(ddev, &xdna->dev_lock); init_rwsem(&xdna->notifier_lock); INIT_LIST_HEAD(&xdna->client_list); @@ -390,9 +392,9 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (ret) return ret; - xdna->notifier_wq = alloc_ordered_workqueue("notifier_wq", WQ_MEM_RECLAIM); - if (!xdna->notifier_wq) { - ret = -ENOMEM; + xdna->notifier_wq = drmm_alloc_ordered_workqueue(ddev, "notifier_wq", WQ_MEM_RECLAIM); + if (IS_ERR(xdna->notifier_wq)) { + ret = PTR_ERR(xdna->notifier_wq); goto iommu_fini; } @@ -401,7 +403,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id) mutex_unlock(&xdna->dev_lock); if (ret) { XDNA_ERR(xdna, "Hardware init failed, ret %d", ret); - goto destroy_notifier_wq; + goto iommu_fini; } ret = amdxdna_sysfs_init(xdna); @@ -425,8 +427,6 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id) mutex_lock(&xdna->dev_lock); xdna->dev_info->ops->fini(xdna); mutex_unlock(&xdna->dev_lock); -destroy_notifier_wq: - destroy_workqueue(xdna->notifier_wq); iommu_fini: amdxdna_iommu_fini(xdna); return ret; @@ -437,23 +437,19 @@ static void amdxdna_remove(struct pci_dev *pdev) struct amdxdna_dev *xdna = pci_get_drvdata(pdev); struct amdxdna_client *client; - destroy_workqueue(xdna->notifier_wq); - 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 34271c14d359f8..a997d27a504dcf 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; diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index 53bb40e70b2778..364cbf79ad7342 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -97,6 +97,9 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences) { int i, j; + if (!num_fences) + return 0; + sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL); /* diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index b3bfa6943a8e13..87797bea91cbdd 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -1168,7 +1168,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence) /* RCU protection is required for safe access to returned string */ ops = rcu_dereference(fence->ops); - if (!dma_fence_test_signaled_flag(fence)) + if (ops) return (const char __rcu *)ops->get_driver_name(fence); else return (const char __rcu *)"detached-driver"; @@ -1201,8 +1201,8 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence) /* RCU protection is required for safe access to returned string */ ops = rcu_dereference(fence->ops); - if (!dma_fence_test_signaled_flag(fence)) - return (const char __rcu *)ops->get_driver_name(fence); + if (ops) + return (const char __rcu *)ops->get_timeline_name(fence); else return (const char __rcu *)"signaled-timeline"; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 4c732e0f776e12..9014678d75abff 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -508,6 +508,7 @@ static int acp_hw_fini(struct amdgpu_ip_block *ip_block) u32 val = 0; u32 count = 0; struct amdgpu_device *adev = ip_block->adev; + int ret = 0; /* return early if no ACP */ if (!adev->acp.acp_genpd) { @@ -529,7 +530,8 @@ static int acp_hw_fini(struct amdgpu_ip_block *ip_block) break; if (--count == 0) { dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); - return -ETIMEDOUT; + ret = -ETIMEDOUT; + goto out; } udelay(100); } @@ -546,21 +548,24 @@ static int acp_hw_fini(struct amdgpu_ip_block *ip_block) break; if (--count == 0) { dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); - return -ETIMEDOUT; + ret = -ETIMEDOUT; + goto out; } udelay(100); } - +out: device_for_each_child(adev->acp.parent, NULL, acp_genpd_remove_device); mfd_remove_devices(adev->acp.parent); kfree(adev->acp.i2s_pdata); kfree(adev->acp.acp_res); + pm_genpd_remove(&adev->acp.acp_genpd->gpd); kfree(adev->acp.acp_genpd); + adev->acp.acp_genpd = NULL; kfree(adev->acp.acp_cell); - return 0; + return ret; } static int acp_suspend(struct amdgpu_ip_block *ip_block) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index c2e6495a28bc58..e714cee2997aa9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1322,7 +1322,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, e->range = NULL; } - if (r || !list_empty(&vm->individual.moved)) { + if (r || !list_empty(&vm->individual.needs_update)) { r = -EAGAIN; mutex_unlock(&p->adev->notifier_lock); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 211d30f03d25f1..8d6502a9430671 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4184,8 +4184,6 @@ static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev) iounmap(adev->rmmio); adev->rmmio = NULL; - if (adev->mman.aper_base_kaddr) - iounmap(adev->mman.aper_base_kaddr); adev->mman.aper_base_kaddr = NULL; /* Memory manager related */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c index be5069642a9076..853365dee2a791 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c @@ -2119,6 +2119,8 @@ static int amdgpu_discovery_set_common_ip_blocks(struct amdgpu_device *adev) case IP_VERSION(11, 5, 3): case IP_VERSION(11, 5, 4): case IP_VERSION(11, 5, 6): + case IP_VERSION(11, 7, 0): + case IP_VERSION(11, 7, 1): amdgpu_device_ip_block_add(adev, &soc21_common_ip_block); break; case IP_VERSION(12, 0, 0): @@ -2180,6 +2182,8 @@ static int amdgpu_discovery_set_gmc_ip_blocks(struct amdgpu_device *adev) case IP_VERSION(11, ... [truncated]