[Devel] [PATCH RH8 03/11] venetdev: fix race between veip shutdown and add veip entry
Andrey Zhadchenko
andrey.zhadchenko at virtuozzo.com
Fri Jun 4 17:45:30 MSK 2021
From: Konstantin Khorenko <khorenko at virtuozzo.com>
If veip entry is added via cgroup ve::ip_allow interface
after veip_shutdown() is called, veip structure leaks.
Moreover, you won't be able to assign same IP address to
a Container (same after restart or another one) because
veip entry exists, but veip->veid mismatches.
This happens because veip_entry_add() creates new veip struct
if ve->veip == NULL. The intention was to handle the case when
we load vznetdev module after a Container start.
In fact this code does not work now: venet device is create by
venet_newlink() which does it only in case env->veip == NULL,
so venet device will be never created in this case.
Let's make veip_entry_add() to fail in case ve->veip == NULL
instead.
Let's also add a warning on veip struct leak. Just in case.
=======================================
Note: we come veip_put() from two paths:
1) __veip_stop()->veip_release(), where ip_lh is definitely empty,
2) venet_exit()->veip_cleanup(), which is called on module unload,
on module unload. Module may be unloaded only after all veip were
destroyed, i.e., went thru (1) path.
https://jira.sw.ru/browse/PSBM-90395
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
Reviewed-by: Kirill Tkhai <ktkhai at virtuozzo.com>
(cherry-picked from 696f833c575ba3a176eecdbe3ad607256ea36397)
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
diff --git a/drivers/net/venetdev.c b/drivers/net/venetdev.c
index 9541ac6..d74e104 100644
--- a/drivers/net/venetdev.c
+++ b/drivers/net/venetdev.c
@@ -60,8 +60,10 @@ static void veip_free(struct rcu_head *rcu)
int veip_put(struct veip_struct *veip)
{
- if (!list_empty(&veip->ip_lh))
+ if (!list_empty(&veip->ip_lh)) {
+ WARN_ONCE(1, "Leaking veip structure 0x%p", veip);
return 0;
+ }
list_del(&veip->list);
call_rcu(&veip->rcu, veip_free);
@@ -256,14 +258,12 @@ static int veip_entry_add(struct ve_struct *ve, struct ve_addr_struct *addr)
if (entry == NULL)
return -ENOMEM;
+ spin_lock(&veip_lock);
if (ve->veip == NULL) {
- /* This can happen if we load venet AFTER ve was started */
- err = veip_start(ve);
- if (err < 0)
- goto out;
+ err = -ENODEV;
+ goto out_unlock;
}
- spin_lock(&veip_lock);
found = venet_entry_lookup(addr);
if (found != NULL) {
err = veip_entry_conflict(found, ve);
@@ -276,9 +276,9 @@ static int veip_entry_add(struct ve_struct *ve, struct ve_addr_struct *addr)
err = 0;
entry = NULL;
+
out_unlock:
spin_unlock(&veip_lock);
-out:
if (entry != NULL)
kfree(entry);
--
1.8.3.1
More information about the Devel
mailing list