lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 16 Sep 2020 15:43:03 -0700
From:   Yonghong Song <yhs@...com>
To:     Martin KaFai Lau <kafai@...com>
CC:     <bpf@...r.kernel.org>, <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, <kernel-team@...com>,
        Song Liu <songliubraving@...com>
Subject: Re: [PATCH bpf-next v3] bpf: using rcu_read_lock for
 bpf_sk_storage_map iterator



On 9/16/20 10:55 AM, Martin KaFai Lau wrote:
> On Tue, Sep 15, 2020 at 11:16:49PM -0700, Yonghong Song wrote:
> [ ... ]
> 
>> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
>> index 4a86ea34f29e..d43c3d6d0693 100644
>> --- a/net/core/bpf_sk_storage.c
>> +++ b/net/core/bpf_sk_storage.c
>> @@ -678,6 +678,7 @@ struct bpf_iter_seq_sk_storage_map_info {
>>   static struct bpf_local_storage_elem *
>>   bpf_sk_storage_map_seq_find_next(struct bpf_iter_seq_sk_storage_map_info *info,
>>   				 struct bpf_local_storage_elem *prev_selem)
>> +	__acquires(RCU) __releases(RCU)
>>   {
>>   	struct bpf_local_storage *sk_storage;
>>   	struct bpf_local_storage_elem *selem;
> In the while loop earlier in this function, if I read it correctly,
> it is sort of continuing the earlier hlist_for_each_entry_rcu() for the
> same bucket, so the hlist_entry_safe() needs to be changed also.
> Something like this (uncompiled code):
> 
>          while (selem) {
> -               selem = hlist_entry_safe(selem->map_node.next,
> +               selem = hlist_entry_safe(rcu_dereference(hlist_next_rcu(&selem->map_node)),
>                                           struct bpf_local_storage_elem, map_node);
>                  if (!selem) {
>                          /* not found, unlock and go to the next bucket */

Thanks and Ack. Will send v4 shortly.

> 
>> @@ -701,11 +702,11 @@ bpf_sk_storage_map_seq_find_next(struct bpf_iter_seq_sk_storage_map_info *info,
>>   		if (!selem) {
>>   			/* not found, unlock and go to the next bucket */
>>   			b = &smap->buckets[bucket_id++];
>> -			raw_spin_unlock_bh(&b->lock);
>> +			rcu_read_unlock();
>>   			skip_elems = 0;
>>   			break;
>>   		}
>> -		sk_storage = rcu_dereference_raw(selem->local_storage);
>> +		sk_storage = rcu_dereference(selem->local_storage);
>>   		if (sk_storage) {
>>   			info->skip_elems = skip_elems + count;
>>   			return selem;

Powered by blists - more mailing lists