mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 03:35:32 -04:00
net: udp_tunnel: fix memory leak in udp_tunnel_nic_unregister()
syzbot reported a memory leak [1] in the UDP tunnel NIC offload code.
When device registration fails (e.g. in register_netdevice()), netdev core
unwinds by sending a single NETDEV_UNREGISTER notification. If work was queued
during NETDEV_REGISTER (utn->work_pending is set), udp_tunnel_nic_unregister()
returns early:
if (utn->work_pending)
return;
Because failed registrations do not enter netdev_wait_allrefs_any(), no
subsequent NETDEV_UNREGISTER rebroadcast will ever occur. As a result, the
struct udp_tunnel_nic allocated in udp_tunnel_nic_alloc() is leaked
permanently.
Fix this by removing the early return. Instead, synchronously cancel any
pending work with cancel_delayed_work_sync() before freeing @utn.
To be able to call cancel_delayed_work_sync() while holding RTNL (the work also
needs RTNL), switch udp_tunnel_nic_device_sync_work() to rtnl_trylock(). If RTNL
is contended, requeue the work with a 1 jiffy delay (via queue_delayed_work())
to prevent high CPU contention while waiting for RTNL lock.
The utn->work_pending bookkeeping is no longer needed and is removed, as
the workqueue core already tracks the pending/running state of the work.
[1]
BUG: memory leak
unreferenced object 0xffff888127d5f840 (size 96):
comm "syz-executor", pid 5806, jiffies 4294942188
backtrace (crc 99fdb6c8):
__kmalloc_noprof+0x3bf/0x550
udp_tunnel_nic_alloc net/ipv4/udp_tunnel_nic.c:756 [inline]
udp_tunnel_nic_register net/ipv4/udp_tunnel_nic.c:833 [inline]
udp_tunnel_nic_netdevice_event+0x804/0xab0 net/ipv4/udp_tunnel_nic.c:931
notifier_call_chain+0x59/0x160 kernel/notifier.c:85
call_netdevice_notifiers_info+0x7d/0xb0 net/core/dev.c:2250
register_netdevice+0xc10/0xeb0 net/core/dev.c:11478
Fixes: cc4e3835ef ("udp_tunnel: add central NIC RX port offload infrastructure")
Reported-by: syzbot+eca845fb8c18dd6b44c1@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a632b15.dde6c935.cf6c8.0011.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260724091137.1792543-1-edumazet@google.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
committed by
Paolo Abeni
parent
88c17de85d
commit
080695e6f0
@@ -32,13 +32,12 @@ struct udp_tunnel_nic_table_entry {
|
||||
* @lock: protects all fields
|
||||
* @need_sync: at least one port start changed
|
||||
* @need_replay: space was freed, we need a replay of all ports
|
||||
* @work_pending: @work is currently scheduled
|
||||
* @n_tables: number of tables under @entries
|
||||
* @missed: bitmap of tables which overflown
|
||||
* @entries: table of tables of ports currently offloaded
|
||||
*/
|
||||
struct udp_tunnel_nic {
|
||||
struct work_struct work;
|
||||
struct delayed_work work;
|
||||
|
||||
struct net_device *dev;
|
||||
|
||||
@@ -46,7 +45,6 @@ struct udp_tunnel_nic {
|
||||
|
||||
u8 need_sync:1;
|
||||
u8 need_replay:1;
|
||||
u8 work_pending:1;
|
||||
|
||||
unsigned int n_tables;
|
||||
unsigned long missed;
|
||||
@@ -301,11 +299,10 @@ __udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
|
||||
static void
|
||||
udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
|
||||
{
|
||||
if (!utn->need_sync || utn->work_pending)
|
||||
if (!utn->need_sync)
|
||||
return;
|
||||
|
||||
queue_work(udp_tunnel_nic_workqueue, &utn->work);
|
||||
utn->work_pending = 1;
|
||||
queue_delayed_work(udp_tunnel_nic_workqueue, &utn->work, 0);
|
||||
}
|
||||
|
||||
static bool
|
||||
@@ -731,12 +728,17 @@ udp_tunnel_nic_replay(struct net_device *dev, struct udp_tunnel_nic *utn)
|
||||
static void udp_tunnel_nic_device_sync_work(struct work_struct *work)
|
||||
{
|
||||
struct udp_tunnel_nic *utn =
|
||||
container_of(work, struct udp_tunnel_nic, work);
|
||||
container_of(work, struct udp_tunnel_nic, work.work);
|
||||
|
||||
rtnl_lock();
|
||||
/* We cannot block on RTNL here, otherwise we would deadlock with
|
||||
* udp_tunnel_nic_unregister() calling cancel_delayed_work_sync()
|
||||
* while holding RTNL. Requeue with 1 jiffy delay if RTNL is contended.
|
||||
*/
|
||||
if (!rtnl_trylock()) {
|
||||
queue_delayed_work(udp_tunnel_nic_workqueue, &utn->work, 1);
|
||||
return;
|
||||
}
|
||||
mutex_lock(&utn->lock);
|
||||
|
||||
utn->work_pending = 0;
|
||||
__udp_tunnel_nic_device_sync(utn->dev, utn);
|
||||
|
||||
if (utn->need_replay)
|
||||
@@ -757,7 +759,7 @@ udp_tunnel_nic_alloc(const struct udp_tunnel_nic_info *info,
|
||||
if (!utn)
|
||||
return NULL;
|
||||
utn->n_tables = n_tables;
|
||||
INIT_WORK(&utn->work, udp_tunnel_nic_device_sync_work);
|
||||
INIT_DELAYED_WORK(&utn->work, udp_tunnel_nic_device_sync_work);
|
||||
mutex_init(&utn->lock);
|
||||
|
||||
for (i = 0; i < n_tables; i++) {
|
||||
@@ -901,11 +903,11 @@ udp_tunnel_nic_unregister(struct net_device *dev, struct udp_tunnel_nic *utn)
|
||||
udp_tunnel_nic_flush(dev, utn);
|
||||
udp_tunnel_nic_unlock(dev);
|
||||
|
||||
/* Wait for the work to be done using the state, netdev core will
|
||||
* retry unregister until we give up our reference on this device.
|
||||
/* Make sure no work is running or queued before freeing @utn.
|
||||
* The work handler uses rtnl_trylock(), so it will not deadlock
|
||||
* against the RTNL we are holding here.
|
||||
*/
|
||||
if (utn->work_pending)
|
||||
return;
|
||||
cancel_delayed_work_sync(&utn->work);
|
||||
|
||||
udp_tunnel_nic_free(utn);
|
||||
release_dev:
|
||||
|
||||
Reference in New Issue
Block a user