Re: [PATCH net v3] net: gro_cells: prevent enabling threaded NAPI on gro_cells

["Björn Töpel" <bjorn@kernel.org>]

Naman Gulati <namangulati@google.com> writes:

> gro_cells allocates a per-CPU struct gro_cell with per-CPU queues
> (cell->napi_skbs), per-CPU local_lock_t (cell->bh_lock), and per-CPU
> napi_struct (cell->napi). On !CONFIG_PREEMPT_RT, local_lock_t only
> provides lockdep annotation (and is a complete no-op when
> CONFIG_DEBUG_LOCK_ALLOC is disabled), relying on per-CPU locality and
> disabled BH context for mutual exclusion; it does not provide cross-CPU
> synchronization.
>
> When threaded NAPI is enabled on a device backed by gro_cells (e.g.
> gre0, vxlan, geneve) via sysfs, unbound kernel threads are created for
> each per-CPU NAPI. When gro_cells_receive() on CPU A enqueues a packet
> to cell_A and calls napi_schedule(&cell_A->napi) while holding
> cell_A->bh_lock, the woken kthread can run concurrently on remote CPU B.
>
> When CPU B runs gro_cell_poll(&cell_A->napi) and calls
> __local_lock_nested_bh(&cell_A->bh_lock), lockdep detects that the lock
> is already held by CPU A's task and triggers a warning:
>
>   WARNING: CPU: 1 PID: 377 at net/core/gro_cells.c:66 gro_cell_poll
>   DEBUG_LOCKS_WARN_ON(l->owner)
>   CPU: 1 UID: 0 PID: 377 Comm: napi/gre0-0 Not tainted 7.3.0-rc2
>   Call Trace:
>    <TASK>
>    __local_lock_nested_bh include/linux/local_lock_internal.h:92
>    gro_cell_poll+0x5aa/0x6c0 net/core/gro_cells.c:66
>    napi_threaded_poll+0x39b/0x600 net/core/dev.c:7048
>    kthread+0x71e/0x870 kernel/kthread.c:464
>    ret_from_fork+0x5d/0x90 arch/x86/kernel/process.c:167
>    ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:264
>    </TASK>
>
> On !CONFIG_PREEMPT_RT kernels without lockdep, concurrent execution of
> __skb_queue_tail() on CPU A and __skb_dequeue() on CPU B without
> cross-CPU locks leads to silent queue corruption.
>
> Virtual per-CPU NAPIs cannot support unbound threaded NAPI. While
> gro_cells also sets NAPI_STATE_NO_BUSY_POLL (which skips napi_hash_add()
> and prevents configuring them via Netlink), NAPI_STATE_NO_BUSY_POLL
> cannot be used to reject threaded NAPI in sysfs because drivers such as
> WireGuard set NAPI_STATE_NO_BUSY_POLL on per-peer NAPIs while running
> them in threaded mode by default.
>
> Introduce NAPI_STATE_PERCPU to mark per-CPU NAPIs that cannot run in
> unbound threaded mode. Set NAPI_STATE_PERCPU in gro_cells_init(), and
> fail early with -EOPNOTSUPP in modify_napi_threaded() and
> netdev_nl_napi_set_config() when trying to enable threaded NAPI on
> NAPI_STATE_PERCPU NAPIs, while still allowing disabling (writing 0) as a
> no-op.
>
> Fixes: 25718fdcbdd2 ("net: gro_cells: Use nested-BH locking for gro_cell")
> Fixes: 29863d41bb6e ("net: implement threaded-able napi poll loop support")
> Reported-by: syzbot+f0661448aa9511ce744a@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6aad6d7b.0c43d342.320d00.0001.GAE@google.com
> Signed-off-by: Naman Gulati <namangulati@google.com>

Reviewed-by: Björn Töpel <bjorn@kernel.org>