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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 27 Aug 2020 08:43:22 -0700
From:   Eric Dumazet <>
To:     Jakub Kicinski <>,
        Eric Dumazet <>
        Rob Sherwood <>
Subject: Re: [PATCH net 1/2] net: disable netpoll on fresh napis

On 8/27/20 8:10 AM, Jakub Kicinski wrote:
> On Thu, 27 Aug 2020 00:25:31 -0700 Eric Dumazet wrote:
>> On 8/26/20 12:40 PM, Jakub Kicinski wrote:
>>> To ensure memory ordering is correct we need to use RCU accessors.
>>> +	set_bit(NAPI_STATE_NPSVC, &napi->state);
>>> +	list_add_rcu(&napi->dev_list, &dev->napi_list);
>>> -	list_for_each_entry(napi, &dev->napi_list, dev_list) {
>>> +	list_for_each_entry_rcu(napi, &dev->napi_list, dev_list) {
>>>  		if (cmpxchg(&napi->poll_owner, -1, cpu) == -1) {
>>>  			poll_one_napi(napi);
>>>  			smp_store_release(&napi->poll_owner, -1);
>> You added rcu in this patch (without anything in the changelog).
> I mentioned I need it for the barriers, in particular I wanted the
> store release barrier in list_add. Not extremely clean :(

Hmmm, we also have smp_mb__after_atomic()

>> netpoll_poll_dev() uses rcu_dereference_bh(), suggesting you might
>> need list_for_each_entry_rcu_bh()
> I thought the RCU flavors are mostly meaningless at this point,
> list_for_each_entry_rcu() checks rcu_read_lock_any_held(). I can add
> the definition of list_for_each_entry_rcu_bh() (since it doesn't exist)
> or go back to non-RCU iteration (since the use is just documentation,
> the code is identical). Or fix it some other way?

Oh, I really thought list_for_each_entry_rcu() was only checking standard rcu.

I might have been confused because we do have hlist_for_each_entry_rcu_bh() helper.

Anyway, when looking at the patch I was not at ease because we do not have proper
rcu grace period when a napi is removed from dev->napi_list. A driver might
free the napi struct right after calling netif_napi_del()

Powered by blists - more mailing lists