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

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
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 <[email protected]>

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/[email protected]?part=11
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.