linux-kernel - Re: [PATCH] drm/meson: fix resource cleanup in meson_drv_bind

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <7c7b3e81-bc46-417b-a3f4-2d48f2d3638d@linaro.org>
Date: Tue, 22 Apr 2025 09:23:34 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
 linux@...tijnvandeventer.nl
Cc: linux-amlogic@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 jbrunet@...libre.com, Furkan Kardame <f.kardame@...jaro.org>
Subject: Re: [PATCH] drm/meson: fix resource cleanup in
 meson_drv_bind_master() on error

On 22/04/2025 09:04, neil.armstrong@...aro.org wrote:
> On 19/04/2025 23:32, Martin Blumenstingl wrote:
>> Hi Martijn, Hi Neil,
>>
>> On Thu, Apr 10, 2025 at 8:46 PM <linux@...tijnvandeventer.nl> wrote:
>>>
>>> Hi Martin,
>>>
>>> Thank you for the patch.
>>>
>>> I encountered this issue some time ago as well and had a possible fix in my tree (see
>>> below).
>>> My apologies for not upstreaming it earlier.
>> No worries, we're all busy with both, offline and online life ;-)
>>
>>> While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It
>>> did make the assumption that only  calling component_unbind_all() was at fault and the the rest of the
>>> code was correct. Therefore, calling one of the following functions:
>>> meson_encoder_dsi_remove()
>>> meson_encoder_hdmi_remove()
>>> meson_encoder_cvbs_remove()
>>> in case their counterpart was not called, should not result in any issues.
>>>
>>> I just verified, and, as far as I understand, all of these functions do a check to confirm
>>> whether the encoder was initialized before proceeding with cleanup.
>> Yep, that seems to be the case right now.
>> Neil, would you like Martijn's more simple approach with a Fixes tag
>> and backport that?
>> Then I'd send my patch as a small cleanup which doesn't have to be
>> backported. Otherwise I'd spin a v2 with a fix for the issue that
>> Martijn found (and including Anand's Reviewed/Tested-by)?
> 
> Please send a follow-up, I'll apply this one today.
> 

Yeah finally please split this in two:
- patch 1 that can be backported
- patch 2 remaining changes

Thanks,
Neil

> Thanks,
> Neil
> 
>>
>> [...]
>>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c
>>>> b/drivers/gpu/drm/meson/meson_drv.c
>>>> index 81d2ee37e773..031686fd4104 100644
>>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>>> @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device
>>>> *dev, bool has_components)
>>>>                        dev_err(drm->dev, "Couldn't bind all
>>>> components\n");
>>>>                        /* Do not try to unbind */
>>>>                        has_components = false;
>>>
>>> Above two lines are no longer used, so can be removed.
>> Well spotted, thank you!
>>
>>
>> Best regards,
>> Martin
>