linux-kernel - Re: [PATCH] drm/panel: novatek-nt35950: Improve error handling

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <df752738-824c-20be-f7d9-83e470912260@linaro.org>
Date:   Mon, 17 Apr 2023 15:38:20 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        neil.armstrong@...aro.org, Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>
Cc:     Marijn Suijten <marijn.suijten@...ainline.org>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: novatek-nt35950: Improve error handling



On 17.04.2023 11:44, AngeloGioacchino Del Regno wrote:
> Il 17/04/23 11:44, AngeloGioacchino Del Regno ha scritto:
>> Il 17/04/23 09:25, Neil Armstrong ha scritto:
>>> On 15/04/2023 13:00, Konrad Dybcio wrote:
>>>> In a very peculiar case when probing and registering with the secondary
>>>> DSI host succeeds, but the OF backlight or DSI attachment fails, the
>>>> primary DSI device is automatically cleaned up, but the secondary one
>>>> is not, leading to -EEXIST when the driver core tries to handle
>>>> -EPROBE_DEFER.
>>>>
>>>> Unregister the DSI1 device manually on failure to prevent that.
>>>>
>>>> Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC panels")
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
>>>> ---
>>>>   drivers/gpu/drm/panel/panel-novatek-nt35950.c | 8 +++++++-
>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
>>>> index abf752b36a52..7498fc6258bb 100644
>>>> --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
>>>> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
>>>> @@ -585,8 +585,11 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
>>>>                  DRM_MODE_CONNECTOR_DSI);
>>>>       ret = drm_panel_of_backlight(&nt->panel);
>>>> -    if (ret)
>>>> +    if (ret) {
>>
>> If this is not a dual-DSI case, nt->dsi[1] will be NULL: I agree it's still
>> fine as in the kernel won't crash in that case, but for logical correctness
>> I would still account for that:
>>
>>          if (num_dsis == 2)
>>              mipi_dsi_device_unregister(nt->dsi[1]);
>>
>>>> +        mipi_dsi_device_unregister(nt->dsi[1]);
>>>> +
>>>>           return dev_err_probe(dev, ret, "Failed to get backlight\n");
>>>> +    }
>>>>       drm_panel_add(&nt->panel);
>>>> @@ -602,6 +605,9 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
>>>>           ret = mipi_dsi_attach(nt->dsi[i]);
>>>>           if (ret < 0) {
>>
>> Same here, please add a check for num_dsis.
Right, I'll send an incremental patch to fix the fix, thanks
for spotting this!

Konrad
>>
>> Regards,
>> Angelo
> 
> Eh, too late, I just noticed that this got picked already...