netdev - [PATCH net] net: openvswitch: fix possible memleak on create vport fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Sep 2019 23:40:08 +0800
From:   xiangxia.m.yue@...il.com
To:     gvrose8192@...il.com, pshelar@....org
Cc:     netdev@...r.kernel.org, Tonghao Zhang <xiangxia.m.yue@...il.com>,
        Taehee Yoo <ap420073@...il.com>
Subject: [PATCH net] net: openvswitch: fix possible memleak on create vport fails

From: Tonghao Zhang <xiangxia.m.yue@...il.com>

If we register a net device which name is not valid
(dev_get_valid_name), register_netdevice will return err
codes and will not run dev->priv_destructor. The memory
will leak. This patch adds check in ovs_vport_free and
set the vport NULL.

Fixes: 309b66970ee2 ("net: openvswitch: do not free vport if register_netdevice() is failed.")
Cc: Taehee Yoo <ap420073@...il.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
---
 net/openvswitch/vport-internal_dev.c | 8 ++------
 net/openvswitch/vport.c              | 9 +++++++++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index d2437b5..074c43f 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -159,7 +159,6 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
 	struct internal_dev *internal_dev;
 	struct net_device *dev;
 	int err;
-	bool free_vport = true;
 
 	vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
 	if (IS_ERR(vport)) {
@@ -190,10 +189,8 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
 
 	rtnl_lock();
 	err = register_netdevice(vport->dev);
-	if (err) {
-		free_vport = false;
+	if (err)
 		goto error_unlock;
-	}
 
 	dev_set_promiscuity(vport->dev, 1);
 	rtnl_unlock();
@@ -207,8 +204,7 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
 error_free_netdev:
 	free_netdev(dev);
 error_free_vport:
-	if (free_vport)
-		ovs_vport_free(vport);
+	ovs_vport_free(vport);
 error:
 	return ERR_PTR(err);
 }
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 3fc38d1..281259a 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -157,11 +157,20 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
  */
 void ovs_vport_free(struct vport *vport)
 {
+	/* We should check whether vport is NULL.
+	 * We may free it again, for example in internal_dev_create
+	 * if register_netdevice fails, vport may have been freed via
+	 * internal_dev_destructor.
+	 */
+	if (unlikely(!vport))
+		return;
+
 	/* vport is freed from RCU callback or error path, Therefore
 	 * it is safe to use raw dereference.
 	 */
 	kfree(rcu_dereference_raw(vport->upcall_portids));
 	kfree(vport);
+	vport = NULL;
 }
 EXPORT_SYMBOL_GPL(ovs_vport_free);
 
-- 
1.8.3.1