[PATCH V3] drm/xe/guc: Fix dereference before NULL check

[PATCH V3] drm/xe/guc: Fix dereference before NULL check

[Everest K.C.]

The pointer list->list is dereferenced before the NULL check.
Fix this by moving the NULL check outside the for loop, so that
the check is performed before the dereferencing.
The list->list pointer cannot be NULL so this has no effect on runtime.
It's just a correctness issue.

This issue was reported by Coverity Scan.
https://scan7.scan.coverity.com/#/project-view/51525/11354?selectedIssue=1600335

Fixes: a18c696fa5cb ("drm/xe/guc: Fix dereference before Null check")
Signed-off-by: Everest K.C. <everestkc@everestkc.com.np>
---
V2 -> V3: - Changed Null to NULL in the changelog
          - Corrected typo in the changelong
          - Added more description to the changelong
	  - Fixed the link for Coverity Report
	  - Removed the space after the Fixes tag
V1 -> V2: - Combined the `!list->list` check in preexisting if statement
	  - Added Fixes tag 
	  - Added the link to the Coverity Scan report 

 drivers/gpu/drm/xe/xe_guc_capture.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
index 41262bda20ed..947c3a6d0e5a 100644
--- a/drivers/gpu/drm/xe/xe_guc_capture.c
+++ b/drivers/gpu/drm/xe/xe_guc_capture.c
@@ -1531,7 +1531,7 @@ read_reg_to_node(struct xe_hw_engine *hwe, const struct __guc_mmio_reg_descr_gro
 {
 	int i;
 
-	if (!list || list->num_regs == 0)
+	if (!list || !list->list || list->num_regs == 0)
 		return;
 
 	if (!regs)
@@ -1541,9 +1541,6 @@ read_reg_to_node(struct xe_hw_engine *hwe, const struct __guc_mmio_reg_descr_gro
 		struct __guc_mmio_reg_descr desc = list->list[i];
 		u32 value;
 
-		if (!list->list)
-			return;
-
 		if (list->type == GUC_STATE_CAPTURE_TYPE_ENGINE_INSTANCE) {
 			value = xe_hw_engine_mmio_read32(hwe, desc.reg);
 		} else {
-- 
2.43.0

Re: [PATCH V3] drm/xe/guc: Fix dereference before NULL check

[Dan Carpenter]

On Thu, Oct 10, 2024 at 12:46:34AM -0600, Everest K.C. wrote:
> The pointer list->list is dereferenced before the NULL check.
> Fix this by moving the NULL check outside the for loop, so that
> the check is performed before the dereferencing.
> The list->list pointer cannot be NULL so this has no effect on runtime.
> It's just a correctness issue.
> 
> This issue was reported by Coverity Scan.
> https://scan7.scan.coverity.com/#/project-view/51525/11354?selectedIssue=1600335
> 
> Fixes: a18c696fa5cb ("drm/xe/guc: Fix dereference before Null check")
> Signed-off-by: Everest K.C. <everestkc@everestkc.com.np>
> ---

Perfect!  Thanks.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH V3] drm/xe/guc: Fix dereference before NULL check

[Michal Wajdeczko]

On 10.10.2024 08:54, Dan Carpenter wrote:
> On Thu, Oct 10, 2024 at 12:46:34AM -0600, Everest K.C. wrote:
>> The pointer list->list is dereferenced before the NULL check.
>> Fix this by moving the NULL check outside the for loop, so that
>> the check is performed before the dereferencing.
>> The list->list pointer cannot be NULL so this has no effect on runtime.
>> It's just a correctness issue.
>>
>> This issue was reported by Coverity Scan.
>> https://scan7.scan.coverity.com/#/project-view/51525/11354?selectedIssue=1600335
>>
>> Fixes: a18c696fa5cb ("drm/xe/guc: Fix dereference before Null check")

hmm, this seems wrong, shouldn't this be:

Fixes: 0f1fdf559225 ("drm/xe/guc: Save manual engine capture into
capture list")

>> Signed-off-by: Everest K.C. <everestkc@everestkc.com.np>
>> ---
> 
> Perfect!  Thanks.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> regards,
> dan carpenter
> 

Re: [PATCH V3] drm/xe/guc: Fix dereference before NULL check

[Dong, Zhanjun]

With the "Fixes" tag fixed, looks good to me to make Coverity happy.

Reviewed-by: Zhanjun Dong <zhanjun.dong@intel.com>

Regards,
Zhanjun Dong

On 2024-10-10 5:33 a.m., Michal Wajdeczko wrote:
> 
> 
> On 10.10.2024 08:54, Dan Carpenter wrote:
>> On Thu, Oct 10, 2024 at 12:46:34AM -0600, Everest K.C. wrote:
>>> The pointer list->list is dereferenced before the NULL check.
>>> Fix this by moving the NULL check outside the for loop, so that
>>> the check is performed before the dereferencing.
>>> The list->list pointer cannot be NULL so this has no effect on runtime.
>>> It's just a correctness issue.
>>>
>>> This issue was reported by Coverity Scan.
>>> https://scan7.scan.coverity.com/#/project-view/51525/11354?selectedIssue=1600335
>>>
>>> Fixes: a18c696fa5cb ("drm/xe/guc: Fix dereference before Null check")
> 
> hmm, this seems wrong, shouldn't this be:
> 
> Fixes: 0f1fdf559225 ("drm/xe/guc: Save manual engine capture into
> capture list")
> 
>>> Signed-off-by: Everest K.C. <everestkc@everestkc.com.np>
>>> ---
>>
>> Perfect!  Thanks.
>>
>> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
>>
>> regards,
>> dan carpenter
>>
>