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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <22c97d77-5983-429b-ba0b-ce821a717a0e@quicinc.com>
Date: Tue, 11 Feb 2025 16:19:39 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Marijn Suijten
	<marijn.suijten@...ainline.org>
CC: Ethan Carter Edwards <ethan@...ancedwards.com>,
        Rob Clark
	<robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        David Airlie
	<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        <linux-hardening@...r.kernel.org>
Subject: Re: [PATCH v2] drm/msm/dpu: Fix uninitialized variable



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@...ancedwards.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@...cinc.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,
>>>
>