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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 27 Aug 2020 08:43:22 -0700 From: Eric Dumazet <eric.dumazet@...il.com> To: Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <eric.dumazet@...il.com> Cc: davem@...emloft.net, michael.chan@...adcom.com, netdev@...r.kernel.org, kernel-team@...com, Rob Sherwood <rsher@...com> 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