Re: [PATCH v4 04/14] drm/panic: Return errno codes if panic output fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Memory leak on test failure due to KUNIT_ASSERT_EQ aborting the test without cleanup.
--

commit 9d8cd66dd2ff7109b373380125b5d333a64fb982
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/panic: Return errno codes if panic output fails

This commit modifies the panic output helpers to return errno codes, allowing
detection of invalid panic handling and avoiding display flushes on error.
The KUnit tests are also updated to assert that these new return values
indicate success.

> diff --git a/drivers/gpu/drm/tests/drm_panic_test.c b/drivers/gpu/drm/tests/drm_panic_test.c
> index ad2f3a2f93b6a..fdd77b0cc54c9 100644
> --- a/drivers/gpu/drm/tests/drm_panic_test.c
> +++ b/drivers/gpu/drm/tests/drm_panic_test.c

[ ... ]

> @@ -102,7 +102,8 @@ static void drm_test_panic_screen_user_map(struct kunit *test)
>  	sb->height = params->height;
>  	sb->pitch[0] = params->width * sb->format->cpp[0];
>  
> -	params->draw_screen(sb);
> +	ret = params->draw_screen(sb);
> +	KUNIT_ASSERT_EQ(test, ret, 0);

[Severity: Low]
If KUNIT_ASSERT_EQ() fails here, does this abort the test and bypass the
subsequent vfree(fb) cleanup step?

If draw_screen() returns a non-zero error code (for instance, due to an
unsupported size or missing font), it looks like this will leak the vmalloc'd
framebuffer since the test aborts before the free can execute.

>  	for (i = 0; i < fb_size; i++)
>  		drm_panic_check_color_byte(test, fb[i]);

[ ... ]

> @@ -146,7 +147,8 @@ static void drm_test_panic_screen_user_page(struct kunit *test)
>  	sb->height = params->height;
>  	sb->pitch[0] = params->width * sb->format->cpp[0];
>  
> -	params->draw_screen(sb);
> +	ret = params->draw_screen(sb);
> +	KUNIT_ASSERT_EQ(test, ret, 0);

[Severity: Low]
Similarly, if KUNIT_ASSERT_EQ() fails here, does it abort the test and
bypass the free_pages cleanup block? 

It appears this would cause a leak of both the allocated pages and the pages
pointer array on test failure.

>  	for (p = 0; p < npages; p++) {
>  		int bytes_in_page = (p == npages - 1) ? fb_size - p * PAGE_SIZE : PAGE_SIZE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909085826.64529-1-tzimmermann@suse.de?part=4