Re: [PATCH bpf-next v7 10/12] selftests/bpf: Add BPF iterator tests for resizable hash map

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The BPF iterator test `rhash_iter_test` does not verify that `read()` completed without an error.
--

commit 80a3099ee25442fbc4f994c884097e00ff4bfc97
Author: Mykyta Yatsenko <yatsenko@meta.com>

selftests/bpf: Add BPF iterator tests for resizable hash map

This commit tests basic BPF iterator functionality for BPF_MAP_TYPE_RHASH,
verifying all elements are visited.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhash.c b/tools/testing/selftests/bpf/prog_tests/rhash.c
> index 69686bf69ba57..98bb66907b7f7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/rhash.c
> +++ b/tools/testing/selftests/bpf/prog_tests/rhash.c

[ ... ]

> @@ -89,6 +90,65 @@ static void rhash_map_extra_too_big(void)
>  		close(fd);
>  }
>  
> +static void rhash_iter_test(void)
> +{

[ ... ]

> +	iter_fd = bpf_iter_create(bpf_link__fd(link));
> +	if (!ASSERT_GE(iter_fd, 0, "create_iter"))
> +		goto free_link;
> +
> +	do {
> +		len = read(iter_fd, buf, sizeof(buf));
> +	} while (len > 0);

[Severity: Medium]
Could this loop silently swallow errors if read() returns a negative value?

If read() returns an error, the loop will exit silently. If an error occurs
after all map elements have already been processed by the BPF program, the
subsequent assertions on key_sum and elem_count might still pass, which would
completely mask the failure. 

Would it make sense to add an explicit ASSERT_GE(len, 0, "read") after the
loop to ensure read() completes without error?

> +
> +	ASSERT_EQ(skel->bss->key_sum, expected_key_sum, "key_sum");
> +	ASSERT_EQ(skel->bss->elem_count, 64, "elem_count");
> +
> +	close(iter_fd);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-rhash-v7-0-5b8e05f8630d@meta.com?part=10