[Devel] [PATCH RHEL8 COMMIT] venetdev: fix race between veip shutdown and add veip entry

[Konstantin Khorenko]

The commit is pushed to "branch-rh8-4.18.0-240.1.1.vz8.5.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-240.1.1.vz8.5.40
------>
commit 635f03445c2b9230ed58c0b851b1b1b4fe2667d0
Author: Konstantin Khorenko <khorenko at virtuozzo.com>
Date:   Fri Jun 11 15:08:01 2021 +0300

    venetdev: fix race between veip shutdown and add veip entry
    
    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 vz7 commit 696f833c575b ("venetdev: fix race between veip
    shutdown and add veip entry"))
    
    Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
---
 drivers/net/venetdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/venetdev.c b/drivers/net/venetdev.c
index 412e18ff55e2..72f42a656764 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);