From: Konstantin Khorenko <khore...@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 <khore...@virtuozzo.com>
Reviewed-by: Kirill Tkhai <ktk...@virtuozzo.com>

(cherry-picked from 696f833c575ba3a176eecdbe3ad607256ea36397)
Signed-off-by: Andrey Zhadchenko <andrey.zhadche...@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

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to