linux-kernel - Re: [PATCH] drm/mediatek: Init `ddp_comp` with devm

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: <e3746b1b-fe23-4205-a5f4-2fb8ff6a9fe7@collabora.com>
Date: Tue, 2 Apr 2024 12:10:17 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Douglas Anderson <dianders@...omium.org>,
 Chun-Kuang Hu <chunkuang.hu@...nel.org>,
 Philipp Zabel <p.zabel@...gutronix.de>
Cc: CK Hu <ck.hu@...iatek.com>, Daniel Vetter <daniel@...ll.ch>,
 David Airlie <airlied@...il.com>, "Jason-JH.Lin"
 <jason-jh.lin@...iatek.com>, Matthias Brugger <matthias.bgg@...il.com>,
 Nathan Lu <nathan.lu@...iatek.com>, dri-devel@...ts.freedesktop.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] drm/mediatek: Init `ddp_comp` with devm_kcalloc()

Il 28/03/24 17:22, Douglas Anderson ha scritto:
> In the case where `conn_routes` is true we allocate an extra slot in
> the `ddp_comp` array but mtk_drm_crtc_create() never seemed to
> initialize it in the test case I ran. For me, this caused a later
> crash when we looped through the array in mtk_drm_crtc_mode_valid().
> This showed up for me when I booted with `slub_debug=FZPUA` which
> poisons the memory initially. Without `slub_debug` I couldn't
> reproduce, presumably because the later code handles the value being
> NULL and in most cases (not guaranteed in all cases) the memory the
> allocator returned started out as 0.
> 
> It really doesn't hurt to initialize the array with devm_kcalloc()
> since the array is small and the overhead of initting a handful of
> elements to 0 is small. In general initting memory to zero is a safer
> practice and usually it's suggested to only use the non-initting alloc
> functions if you really need to.
> 
> Let's switch the function to use an allocation function that zeros the
> memory. For me, this avoids the crash.
> 
> Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
> Signed-off-by: Douglas Anderson <dianders@...omium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>

P.S.: I really dislike the dynamic selection stuff, as that's only a partial
solution for something that should've been in DT from day 1 instead.
P.P.S.: I took care of that already - a series is about to come in a few days.

Cheers,
Angelo