Received-SPF: pass (google.com: domain of linux-kernel+bounces-196976-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: multipart/signed;
 boundary=9ed8c5e261fb18701cc301be073920048b7c8d0a92cc9bbfda52553a915a;
 micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: bulk
Mime-Version: 1.0
Date: Fri, 31 May 2024 16:27:34 +0200
Message-Id: <D1NWF7ESKQ0X.25OZ9MTC2DSBS@gmail.com>
Cc: <dri-devel@lists.freedesktop.org>, <linux-tegra@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>, <baijiaju1990@gmail.com>
Subject: Re: [PATCH] drm/tegra: fix a possible null pointer dereference
From: "Thierry Reding" <thierry.reding@gmail.com>
To: "Huai-Yuan Liu" <qq810974084@gmail.com>, <mperttunen@nvidia.com>,
 <airlied@gmail.com>, <daniel@ffwll.ch>, <jonathanh@nvidia.com>
References: <20240531135608.194168-1-qq810974084@gmail.com>
In-Reply-To: <20240531135608.194168-1-qq810974084@gmail.com>

--9ed8c5e261fb18701cc301be073920048b7c8d0a92cc9bbfda52553a915a
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8

On Fri May 31, 2024 at 3:56 PM CEST, Huai-Yuan Liu wrote:
> In malidp_tegra_crtc_reset, new memory is allocated with kzalloc, but=20
> no check is performed. Before calling __drm_atomic_helper_crtc_reset,=20
> mw_state should be checked to prevent possible null pointer dereferene.

Please check that all these variable name references actually are valid.

Also, "dereference".

> Fixes: b7e0b04ae450 ("drm/tegra: Convert to using __drm_atomic_helper_crt=
c_reset() for reset.")
> Signed-off-by: Huai-Yuan Liu <qq810974084@gmail.com>
> ---
>  drivers/gpu/drm/tegra/dc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index be61c9d1a4f0..7648e129c212 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1388,7 +1388,8 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	if (crtc->state)
>  		tegra_crtc_atomic_destroy_state(crtc, crtc->state);
> =20
> -	__drm_atomic_helper_crtc_reset(crtc, &state->base);
> +	if (state)
> +		__drm_atomic_helper_crtc_reset(crtc, &state->base);

Looking at how other drivers do this, they call

	__drm_atomic_helper_crtc_reset(crtc, NULL);

if they fail to allocate a new state. Any reason why we wouldn't want to
do the same thing here?

Thierry

--9ed8c5e261fb18701cc301be073920048b7c8d0a92cc9bbfda52553a915a
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmZZ3lcACgkQ3SOs138+
s6H03Q//UuqlLodE4/rgaDezJNJcSpL48fqNEFNjdpOEm5Z5qwZUUojcBJj0GNaz
QFYs6+i4594xyoSHq9zAsH/ytcMGRr59aFI/YZt0+RLs0gcJKshIRHgUaESdVXD6
q8ZWe+X0C4YyHZDt7rx8JDtllCOQCqMrDLOaIWETAN5u1gJ23gFfgaU1GGnjgoQY
eAvvPRLVpLDNZv6jgV8a2flO4UinNHbB0X+tYlA5LFPnz3vEyBQVEhpt722cCFVQ
CQWYuUIz9pdelRSgDNJ/Fs8z1Nz8o7CAvVHZykFOY3jfgh9O78QKwM0phhTmB9gn
TwaiFlHHf8vwI4JGEnUphoZhZIEi3Qy2C6W8R1ruGjyYZk4mm17vxBVCOsgmrgAN
ptVh9hKMFfR1ke3tj8wZR3zfr+8aQw5IjxkeHjTI8LIlfWEaOL2rE+bjSfcCkh5L
QVvvvbaCtgeXJ7SAPXIeH6LvRbzU49E8POvHyanYm5v4xDRXnt8ZVdh2luV4jxUN
FdjMtQiTu8rFIOtuj85mjD3gwghaPimrC7w/cb4d2fX03R29psvr0Elsx36PJP8y
LXZo9EhHJLixgHQj7fOT5hYTVDDhGbEuKdrTEfOH0se/HiVizeDP4PnkyzjoLRrh
qGQb3mftwXpoyl+hN6iRNNmDh7dcPj9nuK99RTQQ0xNj9zJ3fWI=
=JUB1
-----END PGP SIGNATURE-----

--9ed8c5e261fb18701cc301be073920048b7c8d0a92cc9bbfda52553a915a--