Re: [PATCH 25/26] dma-buf: Add kunit tests for mapping type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dma-buf: kunit: test_match_fail() overwrites args before type mismatch check
--

commit 4b902f505b8acdddc13e192c921c70bcac60644c
Author: Jason Gunthorpe <jgg@ziepe.ca>

dma-buf: Add kunit tests for mapping type

This commit adds KUnit tests in drivers/dma-buf/st-dma-mapping.c to test
dma_buf_match_mapping() matching and priority rules as well as SGT and
dynamic attachment flows.

> diff --git a/drivers/dma-buf/st-dma-mapping.c b/drivers/dma-buf/st-dma-mapping.c
> new file mode 100644
> index 0000000000000..9d0f1e389e15e
> --- /dev/null
> +++ b/drivers/dma-buf/st-dma-mapping.c
> @@ -0,0 +1,385 @@

[ ... ]

> +static void test_match_fail(struct kunit *test)
> +{
> +	struct dma_buf_mapping_match matches[] = { { .type = &mock_type_a } };
> +	struct dma_buf_mapping_match exp[] = { { .type = &mock_type_b } };
> +	struct dma_buf_attachment attach = {};
> +	struct dma_buf_match_args args = {
> +		.attach = &attach,
> +		.imp_matches = matches,
> +		.imp_len = ARRAY_SIZE(matches),
> +	};
> +
> +	/* Zero-length exporter array returns -EINVAL */
> +	KUNIT_EXPECT_EQ(test, dma_buf_match_mapping(&args, NULL, 0), -EINVAL);
> +	KUNIT_EXPECT_FALSE(test, mock_match_called);
> +	KUNIT_EXPECT_FALSE(test, mock_finish_called);
> +
> +	/* Zero-length importer array returns -EINVAL */
> +	args = (struct dma_buf_match_args){ .attach = &attach };
> +	KUNIT_EXPECT_EQ(test,
> +			dma_buf_match_mapping(&args, matches,
> +					      ARRAY_SIZE(matches)),
> +			-EINVAL);
> +	KUNIT_EXPECT_FALSE(test, mock_match_called);
> +	KUNIT_EXPECT_FALSE(test, mock_finish_called);
> +
> +	/* Different types produce no match */
> +	KUNIT_EXPECT_EQ(test,
> +			dma_buf_match_mapping(&args, exp, ARRAY_SIZE(exp)),
> +			-EINVAL);
> +	KUNIT_EXPECT_FALSE(test, mock_match_called);
> +	KUNIT_EXPECT_FALSE(test, mock_finish_called);
> +}

[Severity: Medium]
In drivers/dma-buf/st-dma-mapping.c, test_match_fail() overwrites args with:

	args = (struct dma_buf_match_args){ .attach = &attach };

for the zero-length importer array check, which zeroes args.imp_matches and
args.imp_len.

When the subsequent "Different types produce no match" check calls
dma_buf_match_mapping(&args, exp, ARRAY_SIZE(exp)), args.imp_matches is still
NULL and args.imp_len is still 0. In drivers/dma-buf/dma-buf-mapping.c,
dma_buf_match_mapping() computes imp_end = args->imp_matches + args->imp_len,
so imp == imp_end on entry to the inner loop:

	for (; exp != exp_end; exp++) {
		const struct dma_buf_mapping_match *imp = args->imp_matches;

		for (; imp != imp_end; imp++) {
			if (exp->type != imp->type)
				continue;

Doesn't this bypass the exp->type != imp->type comparison and just re-test a
zero-length importer array instead of testing mismatched types? Should
args.imp_matches and args.imp_len be restored to matches and
ARRAY_SIZE(matches) before the third dma_buf_match_mapping() call?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026053623.202642-1-apopple@nvidia.com?part=25