Re: [RFC PATCH 11/12] drm/fabric: add mutation KUnit tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized pointer in test timeout path causes kernel panic during cleanup
--

--- Patch [11]: [RFC PATCH 11/12] drm/fabric: add mutation KUnit tests ---
commit 4053deb9cc41982c0c7a86f0e14eb969bfd85f18
Author: Konstantin Sinyuk <ksinyuk@kernel.org>

drm/fabric: add mutation KUnit tests

This commit adds KUnit coverage for the drm_fabric object model and
topology-mutation paths. It tests various endpoint and port mutation scenarios,
including concurrent mutations, orphan attach/detach, and error paths.

> diff --git a/drivers/gpu/drm/fabric/drm_fabric_test.c b/drivers/gpu/drm/fabric/drm_fabric_test.c
> index 863b7cf690689..d2c88d1837375 100644
> --- a/drivers/gpu/drm/fabric/drm_fabric_test.c
> +++ b/drivers/gpu/drm/fabric/drm_fabric_test.c
[ ... ]
> +static void drm_fabric_test_attach_register_collision(struct kunit *test)
> +{
[ ... ]
> +	/* B then proceeds and must fail: X is now owned by A. */
> +	KUNIT_EXPECT_GT(test,
> +			wait_for_completion_timeout(&l4->reg_done, msecs_to_jiffies(5000)),
> +			0);
> +	if (!IS_ERR(l4->reg_ep)) {
> +		KUNIT_ASSERT_EQ(test, 0,
> +				kunit_add_action_or_reset(test,
> +							  fabrictest_unregister_endpoint,
> +							  l4->reg_ep));
> +		KUNIT_FAIL(test, "racing registration unexpectedly succeeded");
> +		return;
> +	}
> +
> +	KUNIT_EXPECT_EQ(test, PTR_ERR(l4->reg_ep), -EEXIST);
> +}

[Severity: Medium]
If the registration thread stalls and wait_for_completion_timeout() times out
(returning 0), l4->reg_ep will remain NULL since l4 was allocated with
kunit_kzalloc().

Because IS_ERR(NULL) evaluates to false, wouldn't this execution path enter
the if block and pass NULL to fabrictest_unregister_endpoint() during cleanup?

This would lead to a NULL pointer dereference and a kernel panic when
drm_fabric_endpoint_unregister() attempts to access the pointer.

Should this check use IS_ERR_OR_NULL(l4->reg_ep) instead, or should l4->reg_ep
be initialized to an error pointer before the thread is spawned?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787552412.git.ksinyuk@kernel.org?part=11