On 8/26/20 12:40 PM, Jakub Kicinski wrote:
> napi_disable() makes sure to set the NAPI_STATE_NPSVC bit to prevent
> netpoll from accessing rings before init is complete. However, the
> same is not done for fresh napi instances in netif_napi_add(),
> even though we expect NAPI instances to be added as disabled.
>
> This causes crashes during driver reconfiguration (enabling XDP,
> changing the channel count) - if there is any printk() after
> netif_napi_add() but before napi_enable().
>
> To ensure memory ordering is correct we need to use RCU accessors.
>
> Reported-by: Rob Sherwood <rs...@fb.com>
> Fixes: 2d8bff12699a ("netpoll: Close race condition between poll_one_napi and
> napi_disable")
> Signed-off-by: Jakub Kicinski <k...@kernel.org>
> ---
> net/core/dev.c | 3 ++-
> net/core/netpoll.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d42c9ea0c3c0..95ac7568f693 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6612,12 +6612,13 @@ void netif_napi_add(struct net_device *dev, struct
> napi_struct *napi,
> netdev_err_once(dev, "%s() called with weight %d\n", __func__,
> weight);
> napi->weight = weight;
> - list_add(&napi->dev_list, &dev->napi_list);
> napi->dev = dev;
> #ifdef CONFIG_NETPOLL
> napi->poll_owner = -1;
> #endif
> set_bit(NAPI_STATE_SCHED, &napi->state);
> + set_bit(NAPI_STATE_NPSVC, &napi->state);
> + list_add_rcu(&napi->dev_list, &dev->napi_list);
> napi_hash_add(napi);
> }
> EXPORT_SYMBOL(netif_napi_add);
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 093e90e52bc2..2338753e936b 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -162,7 +162,7 @@ static void poll_napi(struct net_device *dev)
> struct napi_struct *napi;
> int cpu = smp_processor_id();
>
> - 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).
netpoll_poll_dev() uses rcu_dereference_bh(), suggesting you might need
list_for_each_entry_rcu_bh()
