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 <[email protected]>

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/[email protected]?part=25

Reply via email to