Hi,
On 8/8/2022 11:15 PM, Yonghong Song wrote:
>
>
> On 8/6/22 12:40 AM, Hou Tao wrote:
>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>
>> After closing both related link fd and map fd, reading the map
>> iterator fd to ensure it is OK to do so.
>>
>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
>> ---
>> .../selftests/bpf/prog_tests/bpf_iter.c | 90 +++++++++++++++++++
>> 1 file changed, 90 insertions(+)
SNIP
>> + /* Close link and map fd prematurely */
>> + bpf_link__destroy(link);
>> + bpf_object__destroy_skeleton(*skel);
>> + *skel = NULL;
>> +
>> + /* Let kworker to run first */
>
> Which kworker?
Now bpf map is freed through bpf_map_free_deferred() and it is running in the
kworker context. Will be more specific in v2.
>
>> + usleep(100);
>> + /* Sock map is freed after two synchronize_rcu() calls, so wait */
>> + kern_sync_rcu();
>> + kern_sync_rcu();
>
> In btf_map_in_map.c, the comment mentions two kern_sync_rcu()
> is needed for 5.8 and earlier kernel. Other cases in prog_tests/
> directory only has one kern_sync_rcu(). Why we need two
> kern_sync_rcu() for the current kernel?
As tried to explain in the comment, for both sock map and sock storage map, the
used memory is freed two synchronize_rcu(), so if there are not two
kern_sync_rcu() in the test prog, reading the iterator fd will not be able to
trigger the Use-After-Free problem and it will end normally.
>
>> +
>> + /* Read after both map fd and link fd are closed */
>> + while ((len = read(iter_fd, buf, sizeof(buf))) > 0)
>> + ;
>> + ASSERT_GE(len, 0, "read_iterator");
>> +
>> + close(iter_fd);
>> +}
>> +
>> static int read_fd_into_buffer(int fd, char *buf, int size)
>> {
>> int bufleft = size;
>> @@ -827,6 +870,20 @@ static void test_bpf_array_map(void)
>> bpf_iter_bpf_array_map__destroy(skel);
>> }
>> +static void test_bpf_array_map_iter_fd(void)
>> +{
>> + struct bpf_iter_bpf_array_map *skel;
>> +
>> + skel = bpf_iter_bpf_array_map__open_and_load();
>> + if (!ASSERT_OK_PTR(skel, "bpf_iter_bpf_array_map__open_and_load"))
>> + return;
>> +
>> + do_read_map_iter_fd(&skel->skeleton, skel->progs.dump_bpf_array_map,
>> + skel->maps.arraymap1);
>> +
>> + bpf_iter_bpf_array_map__destroy(skel);
>> +}
>> +
> [...]
