Thread (29 messages) flat view 29 messages, 2 authors, 5d ago

Re: [PATCH v4 14/14] drm/panic: Compile KUnit tests as module

From: sashiko-bot@kernel.org
Date: 2026-09-09 09:37:31
Also in: dri-devel, imx, intel-gfx, intel-xe, linux-hyperv

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The KUnit test exit function `drm_test_panic_exit()` fails to clear the global panic description, leaving a dangling pointer that can crash the panic handler.
--

commit 7eb0aee35aa7e3c110129d29f0c30be49ac71dab
Author: Thomas Zimmermann [off-list ref]

drm/panic: Compile KUnit tests as module

This commit fixes the panic-helper KUnit tests dependency and enables them to
be built as a module. It exports necessary interfaces if KUnit is enabled and
adds cleanup logic for the tests.
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/drm_panic_helper.c b/drivers/gpu/drm/drm_panic_helper.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -815,7 +820,7 @@ static int draw_panic_helper_dispatch(struct drm_scanout_buffer *sb,
 	return ret;
 }
 
-static void drm_panic_helper_set_description(const char *description)
+VISIBLE_IF_KUNIT void drm_panic_helper_set_description(const char *description)
 {
 	u32 len;
 
@@ -830,6 +835,7 @@ static void drm_panic_helper_set_description(const char *description)
 		desc_line->len = len;
 	}
 }
+EXPORT_SYMBOL_IF_KUNIT(drm_panic_helper_set_description);
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/tests/drm_panic_helper_test.c b/drivers/gpu/drm/tests/drm_panic_helper_test.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -86,6 +90,11 @@ static int drm_test_panic_init(struct kunit *test)
 	return 0;
 }
 
+static void drm_test_panic_exit(struct kunit *test)
+{
+	drm_panic_helper_set_description(NULL);
+}
[Severity: High]
Does this leave a dangling pointer in the global panic_msg array when the test
module is unloaded?

Looking at drm_panic_helper_set_description() in drm_panic_helper.c, it
checks if the argument is valid before modifying the state:

    if (description) {
        struct drm_panic_line *desc_line = &panic_msg[panic_msg_lines - 1];
        desc_line->txt = description;
        ...
    }

When drm_test_panic_exit() calls drm_panic_helper_set_description(NULL) to
clear the state, the NULL argument is silently ignored. 

Since drm_test_panic_init() originally sets the description to a string
located in the test module's read-only data, the global panic_msg array will
retain a pointer to this memory even after the module is freed.

If a system panic occurs later, could the panic handler fault when trying to
dereference this stale pointer and fail to render the panic screen?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909085826.64529-1-tzimmermann@suse.de?part=14
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help