[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