[PATCH v2] drm/msm/dpu: Fix uninitialized variable

[PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Ethan Carter Edwards]

There is a possibility for an uninitialized *ret* variable to be
returned in some code paths.

Fix this by initializing *ret* to 0.

Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
---
Changes in v2:
- Return explicit 0 when no error occurs
- Add hardening mailing lists
- Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
 			       unsigned int num_planes)
 {
 	unsigned int i;
-	int ret;
 
 	for (i = 0; i < num_planes; i++) {
 		struct drm_plane_state *plane_state = states[i];
@@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
 		    !plane_state->visible)
 			continue;
 
-		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
+		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
 							 state, plane_state);
 		if (ret)
-			break;
+			return ret;
 	}
 
-	return ret;
+	return 0;
 }
 
 static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)

---
base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
change-id: 20250209-dpu-c3fac78fc617

Best regards,
-- 
Ethan Carter Edwards <ethan@ethancedwards.com>

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Dmitry Baryshkov]

On Sun, Feb 09, 2025 at 10:51:54PM -0500, Ethan Carter Edwards wrote:
> There is a possibility for an uninitialized *ret* variable to be
> returned in some code paths.
> 
> Fix this by initializing *ret* to 0.
> 
> Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
> Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> ---
> Changes in v2:
> - Return explicit 0 when no error occurs
> - Add hardening mailing lists
> - Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Abhinav Kumar]

On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
> There is a possibility for an uninitialized *ret* variable to be
> returned in some code paths.
> 
> Fix this by initializing *ret* to 0.
> 
> Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
> Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> ---
> Changes in v2:
> - Return explicit 0 when no error occurs
> - Add hardening mailing lists
> - Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 

Thanks for your patch, this was addressed with

https://patchwork.freedesktop.org/patch/631567/ but since this is better 
I am fine with this, will pick this one up

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>


> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   			       unsigned int num_planes)
>   {
>   	unsigned int i;
> -	int ret;
>   
>   	for (i = 0; i < num_planes; i++) {
>   		struct drm_plane_state *plane_state = states[i];
> @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   		    !plane_state->visible)
>   			continue;
>   
> -		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> +		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
>   							 state, plane_state);
>   		if (ret)
> -			break;
> +			return ret;
>   	}
>   
> -	return ret;
> +	return 0;
>   }
>   
>   static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
> 
> ---
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> change-id: 20250209-dpu-c3fac78fc617
> 
> Best regards,

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Marijn Suijten]

On 2025-02-10 14:14:14, Abhinav Kumar wrote:
> 
> 
> On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
> > There is a possibility for an uninitialized *ret* variable to be
> > returned in some code paths.
> > 
> > Fix this by initializing *ret* to 0.
> > 
> > Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
> > Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
> > Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> > ---
> > Changes in v2:
> > - Return explicit 0 when no error occurs
> > - Add hardening mailing lists
> > - Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> 
> Thanks for your patch, this was addressed with
> 
> https://patchwork.freedesktop.org/patch/631567/ but since this is better 
> I am fine with this, will pick this one up

The `return 0;` in this patch should certainly fix this issue entirely and we
don't need to inline the `int ret` for that, which I think is against mixed
declaration rules anyway?

As far as I understand that's what Dmitry suggested in v1, but he r-b'd it in
this form.  Dmitry, was that intended?

- Marijn

> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> >   			       unsigned int num_planes)
> >   {
> >   	unsigned int i;
> > -	int ret;
> >   
> >   	for (i = 0; i < num_planes; i++) {
> >   		struct drm_plane_state *plane_state = states[i];
> > @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> >   		    !plane_state->visible)
> >   			continue;
> >   
> > -		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> > +		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> >   							 state, plane_state);
> >   		if (ret)
> > -			break;
> > +			return ret;
> >   	}
> >   
> > -	return ret;
> > +	return 0;
> >   }
> >   
> >   static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
> > 
> > ---
> > base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> > change-id: 20250209-dpu-c3fac78fc617
> > 
> > Best regards,
> 

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Dmitry Baryshkov]

On Tue, Feb 11, 2025 at 10:23:54AM +0100, Marijn Suijten wrote:
> On 2025-02-10 14:14:14, Abhinav Kumar wrote:
> > 
> > 
> > On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
> > > There is a possibility for an uninitialized *ret* variable to be
> > > returned in some code paths.
> > > 
> > > Fix this by initializing *ret* to 0.
> > > 
> > > Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
> > > Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
> > > Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> > > ---
> > > Changes in v2:
> > > - Return explicit 0 when no error occurs
> > > - Add hardening mailing lists
> > > - Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
> > > ---
> > >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
> > >   1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > 
> > Thanks for your patch, this was addressed with
> > 
> > https://patchwork.freedesktop.org/patch/631567/ but since this is better 
> > I am fine with this, will pick this one up
> 
> The `return 0;` in this patch should certainly fix this issue entirely and we
> don't need to inline the `int ret` for that, which I think is against mixed
> declaration rules anyway?
> 
> As far as I understand that's what Dmitry suggested in v1, but he r-b'd it in
> this form.  Dmitry, was that intended?

I think it should be fine, if the gcc doesn't warn against it.

> 
> - Marijn
> 
> > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> > 
> > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > index 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> > >   			       unsigned int num_planes)
> > >   {
> > >   	unsigned int i;
> > > -	int ret;
> > >   
> > >   	for (i = 0; i < num_planes; i++) {
> > >   		struct drm_plane_state *plane_state = states[i];
> > > @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> > >   		    !plane_state->visible)
> > >   			continue;
> > >   
> > > -		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> > > +		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> > >   							 state, plane_state);
> > >   		if (ret)
> > > -			break;
> > > +			return ret;
> > >   	}
> > >   
> > > -	return ret;
> > > +	return 0;
> > >   }
> > >   
> > >   static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
> > > 
> > > ---
> > > base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> > > change-id: 20250209-dpu-c3fac78fc617
> > > 
> > > Best regards,
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Abhinav Kumar]

On 2/11/2025 4:13 PM, Dmitry Baryshkov wrote:
> On Tue, Feb 11, 2025 at 10:23:54AM +0100, Marijn Suijten wrote:
>> On 2025-02-10 14:14:14, Abhinav Kumar wrote:
>>>
>>>
>>> On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
>>>> There is a possibility for an uninitialized *ret* variable to be
>>>> returned in some code paths.
>>>>
>>>> Fix this by initializing *ret* to 0.
>>>>
>>>> Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
>>>> Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
>>>> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
>>>> ---
>>>> Changes in v2:
>>>> - Return explicit 0 when no error occurs
>>>> - Add hardening mailing lists
>>>> - Link to v1: https://lore.kernel.org/r/20250209-dpu-v1-1-0db666884f70@ethancedwards.com
>>>> ---
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
>>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>
>>> Thanks for your patch, this was addressed with
>>>
>>> https://patchwork.freedesktop.org/patch/631567/ but since this is better
>>> I am fine with this, will pick this one up
>>
>> The `return 0;` in this patch should certainly fix this issue entirely and we
>> don't need to inline the `int ret` for that, which I think is against mixed
>> declaration rules anyway?
>>
>> As far as I understand that's what Dmitry suggested in v1, but he r-b'd it in
>> this form.  Dmitry, was that intended?
> 
> I think it should be fine, if the gcc doesn't warn against it.
> 

Let me test out the compilation while applying and see if it throws any 
errors. If it does, will report here and we can go with the other patch.

>>
>> - Marijn
>>
>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>
>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>> index 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>> @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>>>>    			       unsigned int num_planes)
>>>>    {
>>>>    	unsigned int i;
>>>> -	int ret;
>>>>    
>>>>    	for (i = 0; i < num_planes; i++) {
>>>>    		struct drm_plane_state *plane_state = states[i];
>>>> @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>>>>    		    !plane_state->visible)
>>>>    			continue;
>>>>    
>>>> -		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
>>>> +		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
>>>>    							 state, plane_state);
>>>>    		if (ret)
>>>> -			break;
>>>> +			return ret;
>>>>    	}
>>>>    
>>>> -	return ret;
>>>> +	return 0;
>>>>    }
>>>>    
>>>>    static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
>>>>
>>>> ---
>>>> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
>>>> change-id: 20250209-dpu-c3fac78fc617
>>>>
>>>> Best regards,
>>>
> 

Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Abhinav Kumar]

On 2/11/2025 4:19 PM, Abhinav Kumar wrote:
> 
> 
> On 2/11/2025 4:13 PM, Dmitry Baryshkov wrote:
>> On Tue, Feb 11, 2025 at 10:23:54AM +0100, Marijn Suijten wrote:
>>> On 2025-02-10 14:14:14, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 2/9/2025 7:51 PM, Ethan Carter Edwards wrote:
>>>>> There is a possibility for an uninitialized *ret* variable to be
>>>>> returned in some code paths.
>>>>>
>>>>> Fix this by initializing *ret* to 0.
>>>>>
>>>>> Addresses-Coverity-ID: 1642546 ("Uninitialized scalar variable")
>>>>> Fixes: 774bcfb731765d ("drm/msm/dpu: add support for virtual planes")
>>>>> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Return explicit 0 when no error occurs
>>>>> - Add hardening mailing lists
>>>>> - Link to v1: https://lore.kernel.org/r/20250209-dpu- 
>>>>> v1-1-0db666884f70@ethancedwards.com
>>>>> ---
>>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +++----
>>>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>
>>>> Thanks for your patch, this was addressed with
>>>>
>>>> https://patchwork.freedesktop.org/patch/631567/ but since this is 
>>>> better
>>>> I am fine with this, will pick this one up
>>>
>>> The `return 0;` in this patch should certainly fix this issue 
>>> entirely and we
>>> don't need to inline the `int ret` for that, which I think is against 
>>> mixed
>>> declaration rules anyway?
>>>
>>> As far as I understand that's what Dmitry suggested in v1, but he r- 
>>> b'd it in
>>> this form.  Dmitry, was that intended?
>>
>> I think it should be fine, if the gcc doesn't warn against it.
>>
> 
> Let me test out the compilation while applying and see if it throws any 
> errors. If it does, will report here and we can go with the other patch.
> 

On my end, compilation looks fine, so will pickup this version.

Thanks

Abhinav
>>>
>>> - Marijn
>>>
>>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/ 
>>>>> gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> index 
>>>>> 098abc2c0003cde90ce6219c97ee18fa055a92a5..af3e541f60c303eb5212524e877129359b5ca98c 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>>>> @@ -1164,7 +1164,6 @@ int dpu_assign_plane_resources(struct 
>>>>> dpu_global_state *global_state,
>>>>>                       unsigned int num_planes)
>>>>>    {
>>>>>        unsigned int i;
>>>>> -    int ret;
>>>>>        for (i = 0; i < num_planes; i++) {
>>>>>            struct drm_plane_state *plane_state = states[i];
>>>>> @@ -1173,13 +1172,13 @@ int dpu_assign_plane_resources(struct 
>>>>> dpu_global_state *global_state,
>>>>>                !plane_state->visible)
>>>>>                continue;
>>>>> -        ret = dpu_plane_virtual_assign_resources(crtc, global_state,
>>>>> +        int ret = dpu_plane_virtual_assign_resources(crtc, 
>>>>> global_state,
>>>>>                                 state, plane_state);
>>>>>            if (ret)
>>>>> -            break;
>>>>> +            return ret;
>>>>>        }
>>>>> -    return ret;
>>>>> +    return 0;
>>>>>    }
>>>>>    static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct 
>>>>> dpu_sw_pipe *pipe)
>>>>>
>>>>> ---
>>>>> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
>>>>> change-id: 20250209-dpu-c3fac78fc617
>>>>>
>>>>> Best regards,
>>>>
>>
> 

Re: (subset) [PATCH v2] drm/msm/dpu: Fix uninitialized variable

[Abhinav Kumar]

On Sun, 09 Feb 2025 22:51:54 -0500, Ethan Carter Edwards wrote:
> There is a possibility for an uninitialized *ret* variable to be
> returned in some code paths.
> 
> Fix this by initializing *ret* to 0.
> 
> 


Applied to msm-fixes, thanks!

[1/1] drm/msm/dpu: Fix uninitialized variable
      https://gitlab.freedesktop.org/drm/msm/-/commit/978ca99d6bd8

Best regards,
-- 
Abhinav Kumar <quic_abhinavk@quicinc.com>