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 or Netlink, 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:
__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
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. Just as
gro_cells sets NAPI_STATE_NO_BUSY_POLL because virtual NAPIs cannot
support busy polling, introduce NAPI_STATE_NO_THREADED to indicate that
threaded mode is not supported.
Set NAPI_STATE_NO_THREADED in gro_cells_init(), and fail early with
-EOPNOTSUPP when trying to enable threaded napi on gro_cells netdevs.
Fixes: 25718fdcbdd2 ("net: gro_cells: Use nested-BH locking for gro_cell")
Reported-by: syzbot+f0661448aa9511ce744a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6aad6d7b.0c43d342.320d00.0001.GAE@google.com
Reviewed-by: Kuniyuki Iwashima
Signed-off-by: Naman Gulati
---
include/linux/netdevice.h | 2 ++
net/core/gro_cells.c | 1 +
net/core/net-sysfs.c | 4 +++-
net/core/netdev-genl.c | 3 +++
4 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 87cafc932e9e..951734bc8b36 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -433,6 +433,7 @@ enum {
NAPI_STATE_SCHED_THREADED, /* Napi is currently scheduled in threaded mode */
NAPI_STATE_HAS_NOTIFIER, /* Napi has an IRQ notifier */
NAPI_STATE_THREADED_BUSY_POLL, /* The threaded NAPI poller will busy poll */
+ NAPI_STATE_NO_THREADED, /* Threaded mode is not supported */
};
enum {
@@ -448,6 +449,7 @@ enum {
NAPIF_STATE_SCHED_THREADED = BIT(NAPI_STATE_SCHED_THREADED),
NAPIF_STATE_HAS_NOTIFIER = BIT(NAPI_STATE_HAS_NOTIFIER),
NAPIF_STATE_THREADED_BUSY_POLL = BIT(NAPI_STATE_THREADED_BUSY_POLL),
+ NAPIF_STATE_NO_THREADED = BIT(NAPI_STATE_NO_THREADED),
};
enum gro_result {
diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
index d8c0a2867120..9f8884d56f59 100644
--- a/net/core/gro_cells.c
+++ b/net/core/gro_cells.c
@@ -92,6 +92,7 @@ int gro_cells_init(struct gro_cells *gcells, struct net_device *dev)
local_lock_init(&cell->bh_lock);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cell->napi.state);
+ set_bit(NAPI_STATE_NO_THREADED, &cell->napi.state);
netif_napi_add(dev, &cell->napi, gro_cell_poll);
napi_enable(&cell->napi);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 352173df7578..296fdc007862 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -743,9 +743,11 @@ static ssize_t threaded_show(struct device *dev,
static int modify_napi_threaded(struct net_device *dev, unsigned long val)
{
+ struct napi_struct *napi;
int ret;
- if (list_empty(&dev->napi_list))
+ napi = list_first_entry_or_null(&dev->napi_list, typeof(*napi), dev_list);
+ if (!napi || test_bit(NAPI_STATE_NO_THREADED, &napi->state))
return -EOPNOTSUPP;
if (val != 0 && val != 1)
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index cb18db681640..661f87e2f015 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -334,6 +334,9 @@ netdev_nl_napi_set_config(struct napi_struct *napi, struct genl_info *info)
if (info->attrs[NETDEV_A_NAPI_THREADED]) {
int ret;
+ if (test_bit(NAPI_STATE_NO_THREADED, &napi->state))
+ return -EOPNOTSUPP;
+
threaded = nla_get_uint(info->attrs[NETDEV_A_NAPI_THREADED]);
ret = napi_set_threaded(napi, threaded);
if (ret)
--
2.55.0.1082.g2b9226bbc0-goog