From 6e41709eb1d9207d88e46026baf9cc850206b374 Mon Sep 17 00:00:00 2001 From: Hersen Wu Date: Tue, 23 Apr 2024 19:14:16 -0400 Subject: [PATCH] drm/amd/display: Add NULL pointer and OVERRUN check within amdgpu_dm irq register [WHY] Coverity reports OVERRUN issues within amdgpu_dm interrupt registers. Do not check index value before access array. Do not check NULL pointer. [HOW] Add index value check for array. Add check for pointer from amdgpu_dm_irq_register_interrupt. Reviewed-by: Harry Wentland Acked-by: Tom Chung Signed-off-by: Hersen Wu Tested-by: Daniel Wheeler Signed-off-by: Alex Deucher --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 169 +++++++++++++----- 1 file changed, 128 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index b5e5cbbe5e49..b0e50b358720 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3584,7 +3584,7 @@ out: mutex_unlock(&aconnector->hpd_lock); } -static void register_hpd_handlers(struct amdgpu_device *adev) +static int register_hpd_handlers(struct amdgpu_device *adev) { struct drm_device *dev = adev_to_drm(adev); struct drm_connector *connector; @@ -3596,11 +3596,17 @@ static void register_hpd_handlers(struct amdgpu_device *adev) int_params.current_polarity = INTERRUPT_POLARITY_DEFAULT; if (dc_is_dmub_outbox_supported(adev->dm.dc)) { - if (!register_dmub_notify_callback(adev, DMUB_NOTIFICATION_HPD, dmub_hpd_callback, true)) + if (!register_dmub_notify_callback(adev, DMUB_NOTIFICATION_HPD, + dmub_hpd_callback, true)) { DRM_ERROR("amdgpu: fail to register dmub hpd callback"); + return -EINVAL; + } - if (!register_dmub_notify_callback(adev, DMUB_NOTIFICATION_HPD_IRQ, dmub_hpd_callback, true)) + if (!register_dmub_notify_callback(adev, DMUB_NOTIFICATION_HPD_IRQ, + dmub_hpd_callback, true)) { DRM_ERROR("amdgpu: fail to register dmub hpd callback"); + return -EINVAL; + } } list_for_each_entry(connector, @@ -3616,9 +3622,16 @@ static void register_hpd_handlers(struct amdgpu_device *adev) int_params.int_context = INTERRUPT_LOW_IRQ_CONTEXT; int_params.irq_source = dc_link->irq_source_hpd; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - handle_hpd_irq, - (void *) aconnector); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_HPD1 || + int_params.irq_source > DC_IRQ_SOURCE_HPD6) { + DRM_ERROR("Failed to register hpd irq!\n"); + return -EINVAL; + } + + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + handle_hpd_irq, (void *) aconnector)) + return -ENOMEM; } if (dc_link->irq_source_hpd_rx != DC_IRQ_SOURCE_INVALID) { @@ -3627,11 +3640,19 @@ static void register_hpd_handlers(struct amdgpu_device *adev) int_params.int_context = INTERRUPT_LOW_IRQ_CONTEXT; int_params.irq_source = dc_link->irq_source_hpd_rx; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - handle_hpd_rx_irq, - (void *) aconnector); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_HPD1RX || + int_params.irq_source > DC_IRQ_SOURCE_HPD6RX) { + DRM_ERROR("Failed to register hpd rx irq!\n"); + return -EINVAL; + } + + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + handle_hpd_rx_irq, (void *) aconnector)) + return -ENOMEM; } } + return 0; } #if defined(CONFIG_DRM_AMD_DC_SI) @@ -3672,13 +3693,21 @@ static int dce60_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i + 1, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_VBLANK1 || + int_params.irq_source > DC_IRQ_SOURCE_VBLANK6) { + DRM_ERROR("Failed to register vblank irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.vblank_params[int_params.irq_source - DC_IRQ_SOURCE_VBLANK1]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_crtc_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_crtc_high_irq, c_irq_params)) + return -ENOMEM; } /* Use GRPH_PFLIP interrupt */ @@ -3694,14 +3723,21 @@ static int dce60_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_PFLIP_FIRST || + int_params.irq_source > DC_IRQ_SOURCE_PFLIP_LAST) { + DRM_ERROR("Failed to register pflip irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.pflip_params[int_params.irq_source - DC_IRQ_SOURCE_PFLIP_FIRST]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_pflip_high_irq, c_irq_params); - + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_pflip_high_irq, c_irq_params)) + return -ENOMEM; } /* HPD */ @@ -3712,9 +3748,9 @@ static int dce60_register_irq_handlers(struct amdgpu_device *adev) return r; } - register_hpd_handlers(adev); + r = register_hpd_handlers(adev); - return 0; + return r; } #endif @@ -3758,13 +3794,21 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_VBLANK1 || + int_params.irq_source > DC_IRQ_SOURCE_VBLANK6) { + DRM_ERROR("Failed to register vblank irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.vblank_params[int_params.irq_source - DC_IRQ_SOURCE_VBLANK1]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_crtc_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_crtc_high_irq, c_irq_params)) + return -ENOMEM; } /* Use VUPDATE interrupt */ @@ -3779,13 +3823,21 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_VUPDATE1 || + int_params.irq_source > DC_IRQ_SOURCE_VUPDATE6) { + DRM_ERROR("Failed to register vupdate irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.vupdate_params[int_params.irq_source - DC_IRQ_SOURCE_VUPDATE1]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_vupdate_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_vupdate_high_irq, c_irq_params)) + return -ENOMEM; } /* Use GRPH_PFLIP interrupt */ @@ -3801,14 +3853,21 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_PFLIP_FIRST || + int_params.irq_source > DC_IRQ_SOURCE_PFLIP_LAST) { + DRM_ERROR("Failed to register pflip irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.pflip_params[int_params.irq_source - DC_IRQ_SOURCE_PFLIP_FIRST]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_pflip_high_irq, c_irq_params); - + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_pflip_high_irq, c_irq_params)) + return -ENOMEM; } /* HPD */ @@ -3819,9 +3878,9 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) return r; } - register_hpd_handlers(adev); + r = register_hpd_handlers(adev); - return 0; + return r; } /* Register IRQ sources and initialize IRQ callbacks */ @@ -3873,13 +3932,21 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_VBLANK1 || + int_params.irq_source > DC_IRQ_SOURCE_VBLANK6) { + DRM_ERROR("Failed to register vblank irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.vblank_params[int_params.irq_source - DC_IRQ_SOURCE_VBLANK1]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt( - adev, &int_params, dm_crtc_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_crtc_high_irq, c_irq_params)) + return -ENOMEM; } /* Use otg vertical line interrupt */ @@ -3897,9 +3964,11 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, vrtl_int_srcid[i], 0); - if (int_params.irq_source == DC_IRQ_SOURCE_INVALID) { - DRM_ERROR("Failed to register vline0 irq %d!\n", vrtl_int_srcid[i]); - break; + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_DC1_VLINE0 || + int_params.irq_source > DC_IRQ_SOURCE_DC6_VLINE0) { + DRM_ERROR("Failed to register vline0 irq!\n"); + return -EINVAL; } c_irq_params = &adev->dm.vline0_params[int_params.irq_source @@ -3908,8 +3977,10 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_dcn_vertical_interrupt0_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_dcn_vertical_interrupt0_high_irq, + c_irq_params)) + return -ENOMEM; } #endif @@ -3932,13 +4003,21 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_VUPDATE1 || + int_params.irq_source > DC_IRQ_SOURCE_VUPDATE6) { + DRM_ERROR("Failed to register vupdate irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.vupdate_params[int_params.irq_source - DC_IRQ_SOURCE_VUPDATE1]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_vupdate_high_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_vupdate_high_irq, c_irq_params)) + return -ENOMEM; } /* Use GRPH_PFLIP interrupt */ @@ -3955,14 +4034,21 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) int_params.irq_source = dc_interrupt_to_irq_source(dc, i, 0); + if (int_params.irq_source == DC_IRQ_SOURCE_INVALID || + int_params.irq_source < DC_IRQ_SOURCE_PFLIP_FIRST || + int_params.irq_source > DC_IRQ_SOURCE_PFLIP_LAST) { + DRM_ERROR("Failed to register pflip irq!\n"); + return -EINVAL; + } + c_irq_params = &adev->dm.pflip_params[int_params.irq_source - DC_IRQ_SOURCE_PFLIP_FIRST]; c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_pflip_high_irq, c_irq_params); - + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_pflip_high_irq, c_irq_params)) + return -ENOMEM; } /* HPD */ @@ -3973,9 +4059,9 @@ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) return r; } - register_hpd_handlers(adev); + r = register_hpd_handlers(adev); - return 0; + return r; } /* Register Outbox IRQ sources and initialize IRQ callbacks */ static int register_outbox_irq_handlers(struct amdgpu_device *adev) @@ -4006,8 +4092,9 @@ static int register_outbox_irq_handlers(struct amdgpu_device *adev) c_irq_params->adev = adev; c_irq_params->irq_src = int_params.irq_source; - amdgpu_dm_irq_register_interrupt(adev, &int_params, - dm_dmub_outbox1_low_irq, c_irq_params); + if (!amdgpu_dm_irq_register_interrupt(adev, &int_params, + dm_dmub_outbox1_low_irq, c_irq_params)) + return -ENOMEM; } return 0; -- 2.25.1