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

[sashiko-bot@kernel.org]

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 <tzimmermann@suse.de>

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.

> diff --git a/drivers/gpu/drm/drm_panic_helper.c b/drivers/gpu/drm/drm_panic_helper.c
[ ... ]
> @@ -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);
[ ... ]
> diff --git a/drivers/gpu/drm/tests/drm_panic_helper_test.c b/drivers/gpu/drm/tests/drm_panic_helper_test.c
[ ... ]
> @@ -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