From b39610c773431ac7991cf6235e26d693ccabd9e9 Mon Sep 17 00:00:00 2001 From: Donald Robson Date: Fri, 8 Dec 2023 16:08:25 +0000 Subject: [PATCH 1/6] drm/imagination: Fixed infinite loop in pvr_vm_mips_map() Unwinding loop in error path for this function uses unsigned limit variable, causing the promotion of the signed counter variable. --> 204 for (; pfn >= start_pfn; pfn--) ^^^^^^^^^^^^^^^^ If start_pfn can be zero then this is an endless loop. I've seen this code in other places as well. This loop is slightly off as well. It should decrement pfn on the first iteration. Fix by making the loop limit variables signed. Also fix missing predecrement by modifying to while loop. Reported-by: Dan Carpenter Signed-off-by: Donald Robson Signed-off-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20231208160825.92933-1-donald.robson@imgtec.com --- drivers/gpu/drm/imagination/pvr_vm_mips.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/imagination/pvr_vm_mips.c b/drivers/gpu/drm/imagination/pvr_vm_mips.c index 2bc7181a4c3e..b7fef3c797e6 100644 --- a/drivers/gpu/drm/imagination/pvr_vm_mips.c +++ b/drivers/gpu/drm/imagination/pvr_vm_mips.c @@ -152,8 +152,8 @@ pvr_vm_mips_map(struct pvr_device *pvr_dev, struct pvr_fw_object *fw_obj) u64 end; u32 cache_policy; u32 pte_flags; - u32 start_pfn; - u32 end_pfn; + s32 start_pfn; + s32 end_pfn; s32 pfn; int err; @@ -201,7 +201,7 @@ pvr_vm_mips_map(struct pvr_device *pvr_dev, struct pvr_fw_object *fw_obj) return 0; err_unmap_pages: - for (; pfn >= start_pfn; pfn--) + while (--pfn >= start_pfn) WRITE_ONCE(mips_data->pt[pfn], 0); pvr_mmu_flush_request_all(pvr_dev); From f1f55ed3ffe4212f5c96106bf6396c461a2bf223 Mon Sep 17 00:00:00 2001 From: Donald Robson Date: Fri, 8 Dec 2023 16:30:19 +0000 Subject: [PATCH 2/6] drm/imagination: Fixed oops when misusing ioctl CREATE_HWRT_DATASET While writing the matching IGT suite I discovered that it's possible to cause a kernel oops when using DRM_IOCTL_PVR_CREATE_HWRT_DATASET when the call to hwrt_init_common_fw_structure() fails. Use an unwind-type error path to avoid cleaning up the object using the the release function before it is fully resolved. Signed-off-by: Donald Robson Signed-off-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20231208163019.95913-1-donald.robson@imgtec.com --- drivers/gpu/drm/imagination/pvr_hwrt.c | 27 +++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/imagination/pvr_hwrt.c b/drivers/gpu/drm/imagination/pvr_hwrt.c index c4213c18489e..54f88d6c01e5 100644 --- a/drivers/gpu/drm/imagination/pvr_hwrt.c +++ b/drivers/gpu/drm/imagination/pvr_hwrt.c @@ -458,7 +458,7 @@ pvr_hwrt_dataset_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_hwrt_dataset_args *args) { struct pvr_hwrt_dataset *hwrt; - int err; + int err, i = 0; /* Create and fill out the kernel structure */ hwrt = kzalloc(sizeof(*hwrt), GFP_KERNEL); @@ -466,35 +466,36 @@ pvr_hwrt_dataset_create(struct pvr_file *pvr_file, if (!hwrt) return ERR_PTR(-ENOMEM); - kref_init(&hwrt->ref_count); - err = hwrt_init_kernel_structure(pvr_file, args, hwrt); if (err < 0) goto err_free; err = hwrt_init_common_fw_structure(pvr_file, args, hwrt); if (err < 0) - goto err_free; + goto err_fini_kernel_structure; - for (int i = 0; i < ARRAY_SIZE(hwrt->data); i++) { + for (; i < ARRAY_SIZE(hwrt->data); i++) { err = hwrt_data_init_fw_structure(pvr_file, hwrt, args, &args->rt_data_args[i], &hwrt->data[i]); - if (err < 0) { - i--; - /* Destroy already created structures. */ - for (; i >= 0; i--) - hwrt_data_fini_fw_structure(hwrt, i); - goto err_free; - } + if (err < 0) + goto err_fini_data_structures; hwrt->data[i].hwrt_dataset = hwrt; } + kref_init(&hwrt->ref_count); return hwrt; +err_fini_data_structures: + while (--i >= 0) + hwrt_data_fini_fw_structure(hwrt, i); + +err_fini_kernel_structure: + hwrt_fini_kernel_structure(hwrt); + err_free: - pvr_hwrt_dataset_put(hwrt); + kfree(hwrt); return ERR_PTR(err); } From f175498378bdae2ebcf61170a2a866cb96e8a69a Mon Sep 17 00:00:00 2001 From: Donald Robson Date: Wed, 13 Dec 2023 14:44:30 +0000 Subject: [PATCH 3/6] drm/imagination: Fix ERR_PTR test on pointer to pointer. drivers/gpu/drm/imagination/pvr_vm.c:631 pvr_vm_create_context() error: 'vm_ctx->mmu_ctx' dereferencing possible ERR_PTR() 612 vm_ctx->mmu_ctx = pvr_mmu_context_create(pvr_dev); 613 err = PTR_ERR_OR_ZERO(&vm_ctx->mmu_ctx); ^ The address is never an error pointer so this will always return 0. Remove the ampersand. Reported-by: Dan Carpenter Signed-off-by: Donald Robson Signed-off-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20231213144431.94956-1-donald.robson@imgtec.com --- drivers/gpu/drm/imagination/pvr_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c index 82690cee978c..598d79d7c7d0 100644 --- a/drivers/gpu/drm/imagination/pvr_vm.c +++ b/drivers/gpu/drm/imagination/pvr_vm.c @@ -568,7 +568,7 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context) 0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops); vm_ctx->mmu_ctx = pvr_mmu_context_create(pvr_dev); - err = PTR_ERR_OR_ZERO(&vm_ctx->mmu_ctx); + err = PTR_ERR_OR_ZERO(vm_ctx->mmu_ctx); if (err) { vm_ctx->mmu_ctx = NULL; goto err_put_ctx; From 8a53e29fe05c56f643eaab285f224c09b9c3dd4c Mon Sep 17 00:00:00 2001 From: Donald Robson Date: Wed, 13 Dec 2023 14:44:31 +0000 Subject: [PATCH 4/6] drm/imagination: Fix error path in pvr_vm_create_context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is possible to double free the vm_ctx->mmu_ctx object in this function.     630 err_page_table_destroy: --> 631         pvr_mmu_context_destroy(vm_ctx->mmu_ctx); The pvr_vm_context_put() function does:         kref_put(&vm_ctx->ref_count, pvr_vm_context_release); Here the pvr_vm_context_release() will call:         pvr_mmu_context_destroy(vm_ctx->mmu_ctx); Refactor to an unwind style. Reported-by: Dan Carpenter Signed-off-by: Donald Robson Reviewed-by: Dan Carpenter Signed-off-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20231213144431.94956-2-donald.robson@imgtec.com --- drivers/gpu/drm/imagination/pvr_vm.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c index 598d79d7c7d0..e59517ba039e 100644 --- a/drivers/gpu/drm/imagination/pvr_vm.c +++ b/drivers/gpu/drm/imagination/pvr_vm.c @@ -556,23 +556,12 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context) if (!vm_ctx) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0); - vm_ctx->pvr_dev = pvr_dev; - kref_init(&vm_ctx->ref_count); - mutex_init(&vm_ctx->lock); - - drm_gpuvm_init(&vm_ctx->gpuvm_mgr, - is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM", - 0, &pvr_dev->base, &vm_ctx->dummy_gem, - 0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops); vm_ctx->mmu_ctx = pvr_mmu_context_create(pvr_dev); err = PTR_ERR_OR_ZERO(vm_ctx->mmu_ctx); - if (err) { - vm_ctx->mmu_ctx = NULL; - goto err_put_ctx; - } + if (err) + goto err_free; if (is_userspace_context) { err = pvr_fw_object_create(pvr_dev, sizeof(struct rogue_fwif_fwmemcontext), @@ -583,13 +572,22 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context) goto err_page_table_destroy; } + drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0); + drm_gpuvm_init(&vm_ctx->gpuvm_mgr, + is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM", + 0, &pvr_dev->base, &vm_ctx->dummy_gem, + 0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops); + + mutex_init(&vm_ctx->lock); + kref_init(&vm_ctx->ref_count); + return vm_ctx; err_page_table_destroy: pvr_mmu_context_destroy(vm_ctx->mmu_ctx); -err_put_ctx: - pvr_vm_context_put(vm_ctx); +err_free: + kfree(vm_ctx); return ERR_PTR(err); } From 6914968a0b52507bf19d85e5fb9e35272e17cd35 Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Sun, 17 Dec 2023 01:59:10 +0200 Subject: [PATCH 5/6] drm/bridge: properly refcount DT nodes in aux bridge drivers The aux-bridge and aux-hpd-bridge drivers didn't call of_node_get() on the device nodes further used for dev->of_node and platform data. When bridge devices are released, the reference counts are decreased, resulting in refcount underflow / use-after-free warnings. Get corresponding refcounts during AUX bridge allocation. Reported-by: Luca Weiss Fixes: 2a04739139b2 ("drm/bridge: add transparent bridge helper") Fixes: 26f4bac3d884 ("drm/bridge: aux-hpd: Replace of_device.h with explicit include") Reviewed-by: Neil Armstrong Signed-off-by: Dmitry Baryshkov Link: https://patchwork.freedesktop.org/patch/msgid/20231216235910.911958-1-dmitry.baryshkov@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/bridge/aux-bridge.c | 3 ++- drivers/gpu/drm/bridge/aux-hpd-bridge.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c index 49d7c2ab1ecc..b29980f95379 100644 --- a/drivers/gpu/drm/bridge/aux-bridge.c +++ b/drivers/gpu/drm/bridge/aux-bridge.c @@ -6,6 +6,7 @@ */ #include #include +#include #include #include @@ -57,7 +58,7 @@ int drm_aux_bridge_register(struct device *parent) adev->id = ret; adev->name = "aux_bridge"; adev->dev.parent = parent; - adev->dev.of_node = parent->of_node; + adev->dev.of_node = of_node_get(parent->of_node); adev->dev.release = drm_aux_bridge_release; ret = auxiliary_device_init(adev); diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c index 1999a053d59b..bb55f697a181 100644 --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c @@ -68,9 +68,9 @@ struct device *drm_dp_hpd_bridge_register(struct device *parent, adev->id = ret; adev->name = "dp_hpd_bridge"; adev->dev.parent = parent; - adev->dev.of_node = parent->of_node; + adev->dev.of_node = of_node_get(parent->of_node); adev->dev.release = drm_aux_hpd_bridge_release; - adev->dev.platform_data = np; + adev->dev.platform_data = of_node_get(np); ret = auxiliary_device_init(adev); if (ret) { From 933a2a376fb3f22ba4774f74233571504ac56b02 Mon Sep 17 00:00:00 2001 From: Stephen Rothwell Date: Tue, 19 Dec 2023 14:57:34 +1100 Subject: [PATCH 6/6] drm: using mul_u32_u32() requires linux/math64.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some pending include file cleanups produced this error: In file included from include/linux/kernel.h:27, from drivers/gpu/ipu-v3/ipu-dp.c:7: include/drm/drm_color_mgmt.h: In function 'drm_color_lut_extract': include/drm/drm_color_mgmt.h:45:46: error: implicit declaration of function 'mul_u32_u32' [-Werror=implicit-function-declaration] 45 | return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(user_input, (1 << bit_precision) - 1), | ^~~~~~~~~~~ Fixes: c6fbb6bca108 ("drm: Fix color LUT rounding") Signed-off-by: Stephen Rothwell Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20231219145734.13e40e1e@canb.auug.org.au --- include/drm/drm_color_mgmt.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 54b2b2467bfd..ed81741036d7 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -24,6 +24,7 @@ #define __DRM_COLOR_MGMT_H__ #include +#include #include struct drm_crtc;