linux-kernel - [PATCH] drm/atmel-hlcdc: Release CRTC commit when destroying plane state

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20240306194935.11871-1-pl.dourneau@klervi.com>
Date: Wed,  6 Mar 2024 20:49:35 +0100
From: Pierre-Louis Dourneau <pl.dourneau@...rvi.com>
To: Sam Ravnborg <sam@...nborg.org>,
	Boris Brezillon <bbrezillon@...nel.org>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>,
	Daniel Vetter <daniel@...ll.ch>,
	Nicolas Ferre <nicolas.ferre@...rochip.com>,
	Alexandre Belloni <alexandre.belloni@...tlin.com>,
	Claudiu Beznea <claudiu.beznea@...on.dev>,
	dri-devel@...ts.freedesktop.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	Manikandan Muralidharan <manikandan.m@...rochip.com>
Cc: Pierre-Louis Dourneau <pl.dourneau@...rvi.com>,
	Benoît Alcaina <b.alcaina@...rvi.com>,
	Arnaud Lahache <a.lahache@...rvi.com>,
	Nicolas Georges <n.georges@...rvi.com>
Subject: [PATCH] drm/atmel-hlcdc: Release CRTC commit when destroying plane state

From: Arnaud Lahache <a.lahache@...rvi.com>

Fixes a memory leak occurring on each modeset update.

Running a program such as libdrm's modetest[0] with this driver exhausts
all available memory after a few minutes. Enabling kmemleak yields a series
of such leak reports:

unreferenced object 0xc21acf40 (size 64):
  comm "modetest", pid 210, jiffies 4294942460 (age 331.240s)
  hex dump (first 32 bytes):
    00 a0 a1 c1 01 00 00 00 ff ff ff ff 4c cf 1a c2  ............L...
    4c cf 1a c2 ff ff ff ff 58 cf 1a c2 58 cf 1a c2  L.......X...X...
  backtrace:
    [<d68b3e09>] kmalloc_trace+0x18/0x24
    [<f858a020>] drm_atomic_helper_setup_commit+0x1e0/0x7e0
    [<26e8ab04>] drm_atomic_helper_commit+0x40/0x160
    [<49708b0c>] drm_atomic_commit+0xa8/0xf0
    [<e58c2942>] drm_mode_obj_set_property_ioctl+0x154/0x3d8
    [<5e97e57d>] drm_ioctl+0x200/0x3c4
    [<ed514ba1>] sys_ioctl+0x240/0xb48
    [<26aab344>] ret_fast_syscall+0x0/0x44

drm_atomic_helper_setup_commit() acquires a reference to a drm_crtc_commit
for each CRTC and associated connectors and planes involved in a modeset.
64-byte leaks map well to the size of a drm_crtc_commit on 32-bit
architectures, and the second 4-byte chunk in the hex dump above awfully
looks like a reference count.

We tracked this missing reference decrement down to the driver's
atmel_hlcdc_plane_atomic_destroy_state() callback. Its CRTC counterpart,
atmel_hlcdc_crtc_destroy_state(), calls into the drm_atomic helpers and
properly releases its references to the commit. Planes didn't. Using the
default helper for that purpose, __drm_atomic_helper_plane_destroy_state(),
fixes the leak and avoids reimplementing the same logic in the driver.

[0]: https://gitlab.freedesktop.org/mesa/drm/-/tree/main/tests/modetest
     Invoke with `modetest -M atmel-hlcdc -s 32:#0 -v`, assuming 32 is the
     ID of a connector.

Signed-off-by: Arnaud Lahache <a.lahache@...rvi.com>
Co-developed-by: Pierre-Louis Dourneau <pl.dourneau@...rvi.com>
Signed-off-by: Pierre-Louis Dourneau <pl.dourneau@...rvi.com>
Co-developed-by: Benoît Alcaina <b.alcaina@...rvi.com>
Signed-off-by: Benoît Alcaina <b.alcaina@...rvi.com>
---
As far as our testing goes, we've been running 6 of our production units
with this patch for more than 2 weeks as per the date this patch is sent
out. We can report stable memory usage.

Admittedly, our usage of the DRM uAPI is rather simple: create 2 dumb
buffers, do an initial MODE_SETCRTC, and then MODE_PAGE_FLIP between the
two dumb buffers at the rate of once per second on average. We haven't
evaluated more complex workloads.

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index daa508504f47..390c4fc62af7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -934,8 +934,7 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
 			      state->dscrs[i]->self);
 	}
 
-	if (s->fb)
-		drm_framebuffer_put(s->fb);
+	__drm_atomic_helper_plane_destroy_state(s);
 
 	kfree(state);
 }

base-commit: 9dfc46c87cdc8f5a42a71de247a744a6b8188980
-- 
2.34.1