[PATCH] drm/amdgpu: Fix variable 'mca_funcs' dereferenced before NULL check in 'amdgpu_mca_smu_get_mca_entry()'

[Wang, Yang(Kevin)]

[AMD Official Use Only - General]

-----Original Message-----
From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
Sent: Thursday, January 4, 2024 10:32 AM
To: Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>
Cc: amd-gfx at lists.freedesktop.org; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>; Wang, Yang(Kevin) <KevinYang.Wang at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
Subject: [PATCH] drm/amdgpu: Fix variable 'mca_funcs' dereferenced before NULL check in 'amdgpu_mca_smu_get_mca_entry()'

Fixes the below:

drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c:377 amdgpu_mca_smu_get_mca_entry() warn: variable dereferenced before check 'mca_funcs' (see line 368)

357 int amdgpu_mca_smu_get_mca_entry(struct amdgpu_device *adev,
                                     enum amdgpu_mca_error_type type,
358                                  int idx, struct mca_bank_entry *entry)
359 {
360         const struct amdgpu_mca_smu_funcs *mca_funcs =
                                                adev->mca.mca_funcs;
361         int count;
362
363         switch (type) {
364         case AMDGPU_MCA_ERROR_TYPE_UE:
365                 count = mca_funcs->max_ue_count;

mca_funcs is dereferenced here.

366                 break;
367         case AMDGPU_MCA_ERROR_TYPE_CE:
368                 count = mca_funcs->max_ce_count;

mca_funcs is dereferenced here.

369                 break;
370         default:
371                 return -EINVAL;
372         }
373
374         if (idx >= count)
375                 return -EINVAL;
376
377         if (mca_funcs && mca_funcs->mca_get_mca_entry)
                ^^^^^^^^^

Checked too late!

Cc: Yang Wang <kevinyang.wang at amd.com>
Cc: Hawking Zhang <Hawking.Zhang at amd.com>
Cc: Christian König <christian.koenig at amd.com>
Cc: Alex Deucher <alexander.deucher at amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
index 8911310f98df..18790d5c96c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
@@ -360,6 +360,9 @@ int amdgpu_mca_smu_get_mca_entry(struct amdgpu_device *adev, enum amdgpu_mca_err
        const struct amdgpu_mca_smu_funcs *mca_funcs = adev->mca.mca_funcs;
        int count;

+       if (!mca_funcs || !mca_funcs->mca_get_mca_entry)
+               return -EOPNOTSUPP;
+
        switch (type) {
        case AMDGPU_MCA_ERROR_TYPE_UE:
                count = mca_funcs->max_ue_count;
@@ -374,7 +377,7 @@ int amdgpu_mca_smu_get_mca_entry(struct amdgpu_device *adev, enum amdgpu_mca_err
        if (idx >= count)
                return -EINVAL;

-       if (mca_funcs && mca_funcs->mca_get_mca_entry)
+       if (mca_funcs->mca_get_mca_entry)
                return mca_funcs->mca_get_mca_entry(adev, type, idx, entry);

[kevin]:

if (mca_funcs->mca_get_mca_entry)
I think you'd better remove this condition because the above code is always true, right ?

With that fix, the patch is.
Reviewed-by: Yang Wang <kevinyang.wang at amd.com>

Best Regards,
Kevin

        return -EOPNOTSUPP;
--
2.34.1