Re: [RFC PATCH 6/6] drm/gma500: Add software-only KUnit coverage for SGX page tables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/gma500: NULL deref and double free in psb_mmu_test_exit()
  on init error
--

commit 0ae7cbd175ba45e2df744da8590157d933a675ae
Author: Gabriel Gama <operatingsystemsdepression@gmail.com>

drm/gma500: Add software-only KUnit coverage for SGX page tables

This commit adds a built-in KUnit test suite for GMA500 SGX page-table
operations using a software page directory without hardware access.

> diff --git a/drivers/gpu/drm/gma500/mmu_test.c b/drivers/gpu/drm/gma500/mmu_test.c
> new file mode 100644
> index 0000000000000..0581c7d9ac4ac
> --- /dev/null
> +++ b/drivers/gpu/drm/gma500/mmu_test.c
> @@ -0,0 +1,222 @@
[ ... ]
> +static int psb_mmu_test_init(struct kunit *test)
> +{
> +	struct psb_mmu_test_context *ctx;
> +	int i;
> +
> +	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	test->priv = ctx;
> +	spin_lock_init(&ctx->driver.lock);
> +	init_rwsem(&ctx->driver.sem);
> +	ctx->pd = psb_mmu_alloc_pd(&ctx->driver, 1, 0);
> +	if (!ctx->pd)
> +		return -ENOMEM;
> +	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++) {
> +		ctx->pages[i] = alloc_page(GFP_DMA32);
> +		if (!ctx->pages[i]) {
> +			while (i--)
> +				__free_page(ctx->pages[i]);
> +			psb_mmu_free_pagedir(ctx->pd);
> +			return -ENOMEM;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static void psb_mmu_test_exit(struct kunit *test)
> +{
> +	struct psb_mmu_test_context *ctx = test->priv;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++)
> +		__free_page(ctx->pages[i]);
> +	psb_mmu_free_pagedir(ctx->pd);
> +}

[Severity: Medium]
Can psb_mmu_test_exit() cause a NULL pointer dereference or double free
when psb_mmu_test_init() fails?

KUnit runs suite->exit(test) via kunit_run_case_cleanup() even when
suite->init(test) returns a non-zero error.  If kunit_kzalloc() or
psb_mmu_alloc_pd() fails, psb_mmu_test_exit() dereferences a NULL ctx or
ctx->pd; if alloc_page() fails, psb_mmu_test_init() frees ctx->pages[] and
ctx->pd without clearing the pointers, and psb_mmu_test_exit() frees them
again while passing NULL for the unallocated pages.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109055451.17458-1-operatingsystemsdepression@gmail.com?part=6