Re: [PATCH v10 8/8] iio: osf: add IIO KUnit tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] kthread: UAF in osf_iio_test_producer() on KUnit assertion failure
- [Medium] mm: missing memory barrier in osf_iio_buffer_lifetime_test() causes test timeout
- [Medium] iio: memory and device leak on KUnit assertion failure
--

commit 8655dd7dbcb0e7d37056c4f127c9412727c35c55
Author: Jinseob Kim <kimjinseob88@gmail.com>

iio: osf: add IIO KUnit tests

Add the existing IIO active-scan packing and buffer producer lifetime
tests. Extend the test build wiring and help text to cover both suites.

> diff --git a/drivers/iio/opensensorfusion/osf_iio_test.c b/drivers/iio/opensensorfusion/osf_iio_test.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_iio_test.c
[ ... ]
> +static void osf_iio_check_scan(struct kunit *test, struct iio_dev *indio_dev,
> +			       const s32 *values, unsigned int channels,
> +			       unsigned int mask, bool ts)
> +{
[ ... ]
> +	buffer->scan_timestamp = ts;
> +	ret = iio_update_buffers(indio_dev, buffer, NULL);
> +	KUNIT_ASSERT_EQ(test, ret, 0);

[Severity: Medium]
If this assertion fails, the test function is aborted instantly.

Since IIO devices and buffers are registered manually using
iio_device_alloc() and iio_device_register(), they require explicit
unregistration via osf_iio_unregister_sensor().

Will bypassing the explicit cleanup permanently leak the IIO device, kfifo,
and associated memory? Should KUnit deferred actions (kunit_add_action) be
used for cleanup instead?

[ ... ]
> +static int osf_iio_test_producer(void *data)
> +{
> +	struct osf_iio_race *race = data;
> +	const s32 values[] = { 101, -202, 303 };
> +
> +	while (!kthread_should_stop()) {
> +		osf_iio_push_sample(race->indio_dev, values, ARRAY_SIZE(values));

[Severity: High]
If a KUNIT_ASSERT_* check fails elsewhere in the test, KUnit will abort the
test and free the race structure as part of test cleanup.

Because the test aborted, kthread_stop() is never called. Will this leave
the producer thread running and accessing the freed race memory, causing a
use-after-free regression?

> +		cond_resched();
> +	}
> +	return 0;
> +}
[ ... ]
> +static void osf_iio_expect_quiesce(struct kunit *test, struct osf_iio_race *race)
> +{
> +	struct task_struct *config;
> +	unsigned long waited;
> +
> +	config = kthread_run(osf_iio_test_configure, race, "osf-cfg-test");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config);

[Severity: High]
If this assertion fails, the test aborts without stopping the running
producer thread. Can this trigger the use-after-free regression mentioned
above?

[ ... ]
> +static void osf_iio_buffer_lifetime_test(struct kunit *test)
> +{
[ ... ]
> +	atomic_set(&race->block_store, 1);
> +	producer = kthread_run(osf_iio_test_producer, race, "osf-rx-test");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, producer);

[Severity: Medium]
If creating the producer thread fails, the test aborts immediately. As with
the assertion in osf_iio_check_scan(), will this leave the IIO sensor
permanently registered and cause a memory leak regression?

[ ... ]
> +	KUNIT_EXPECT_EQ(test, iio_update_buffers(indio_dev, buffer, NULL), 0);
> +	/* Retain ownership while testing unregister against an admitted push. */
> +	get_device(&indio_dev->dev);
> +	reinit_completion(&race->entered);
> +	reinit_completion(&race->release);
> +	reinit_completion(&race->config_started);
> +	reinit_completion(&race->config_done);
> +	atomic_set(&race->block_store, 1);

[Severity: Medium]
Does this code lack a memory barrier before atomic_set(&race->block_store, 1)?

The test uses reinit_completion() which performs a relaxed write to x->done,
followed immediately by the atomic_set().

Could the continuous background producer thread observe block_store == 1 and
execute complete() before the delayed reinit_completion() write takes effect
in the main thread?

If the delayed write overwrites the completion status, will the main thread's
subsequent wait_for_completion_timeout() block indefinitely and cause a test
timeout regression?

> +	KUNIT_EXPECT_NE(test, wait_for_completion_timeout(&race->entered, HZ), 0UL);
> +	race->unregister = true;
> +	osf_iio_expect_quiesce(test, race);
> +	kthread_stop(producer);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789753020.git.kimjinseob88@gmail.com?part=8