linux-kernel - Re: [PATCH] drm/mediatek: dp: drm_err => dev

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: <5f04e7bf5d9618f1a61eb77de63fc1c6731762f1.camel@mediatek.com>
Date: Fri, 7 Feb 2025 03:38:43 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, "dianders@...omium.org"
	<dianders@...omium.org>, "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
CC: "simona@...ll.ch" <simona@...ll.ch>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, "airlied@...il.com"
	<airlied@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "linux-mediatek@...ts.infradead.org"
	<linux-mediatek@...ts.infradead.org>, Alexandre Mergnat
	<amergnat@...libre.com>
Subject: Re: [PATCH] drm/mediatek: dp: drm_err => dev_err in HPD path to avoid
 NULL ptr

Hi, Douglas:

On Thu, 2025-01-16 at 09:42 -0800, Douglas Anderson wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The function mtk_dp_wait_hpd_asserted() may be called before the
> `mtk_dp->drm_dev` pointer is assigned in mtk_dp_bridge_attach().
> Specifically it can be called via this callpath:
>  - mtk_edp_wait_hpd_asserted
>  - [panel probe]
>  - dp_aux_ep_probe
> 
> Using "drm" level prints anywhere in this callpath causes a NULL
> pointer dereference. Change the error message directly in
> mtk_dp_wait_hpd_asserted() to dev_err() to avoid this. Also change the
> error messages in mtk_dp_parse_capabilities(), which is called by
> mtk_dp_wait_hpd_asserted().
> 
> While touching these prints, also add the error code to them to make
> future debugging easier.

Reviewed-by: CK Hu <ck.hu@...iatek.com>

> 
> Fixes: 7eacba9a083b ("drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus")
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> Unfortunately, I have only been able to compile-time test this code. I
> hit the NULL pointer dereference on a device that's nowhere near
> upstream and it was running (sigh) a heavily modified copy of this
> code where the eDP stuff has been forked out of DP. Specifically, you
> can see <https://urldefense.com/v3/__https://crrev.com/c/6073744__;!!CTRNKA9wMg0ARbw!jb1QV1V5hUEEdobvFihzW88qPITWculzska3y5GtFBXuxiiOWeavhqC5GItPDzbQ0Jdiz7hEGpvaOdsCxA$ >. It's pretty easy to understand
> that the same problem affects both codebases though, so I'm posting
> this "blind" in the hopes to at least fix upstream.
> 
> I'll also note that the fact that mtk_edp_wait_hpd_asserted() calls
> mtk_dp_parse_capabilities() feels weird/wrong to me based on other eDP
> code I've worked on, but I've only barely looked at the Mediatek
> driver and perhaps others have already debated this. In any case,
> that's not directly related to this patch.
> 
>  drivers/gpu/drm/mediatek/mtk_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 0687672f0e52..ccd104d8851f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1763,7 +1763,7 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
> 
>         ret = drm_dp_dpcd_readb(&mtk_dp->aux, DP_MSTM_CAP, &val);
>         if (ret < 1) {
> -               drm_err(mtk_dp->drm_dev, "Read mstm cap failed\n");
> +               dev_err(mtk_dp->dev, "Read mstm cap failed: %zd\n", ret);
>                 return ret == 0 ? -EIO : ret;
>         }
> 
> @@ -1773,7 +1773,7 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
>                                         DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
>                                         &val);
>                 if (ret < 1) {
> -                       drm_err(mtk_dp->drm_dev, "Read irq vector failed\n");
> +                       dev_err(mtk_dp->dev, "Read irq vector failed: %zd\n", ret);
>                         return ret == 0 ? -EIO : ret;
>                 }
> 
> @@ -2056,7 +2056,7 @@ static int mtk_dp_wait_hpd_asserted(struct drm_dp_aux *mtk_aux, unsigned long wa
> 
>         ret = mtk_dp_parse_capabilities(mtk_dp);
>         if (ret) {
> -               drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
> +               dev_err(mtk_dp->dev, "Can't parse capabilities: %d\n", ret);
>                 return ret;
>         }
> 
> --
> 2.48.0.rc2.279.g1de40edade-goog
>