net: openvswitch: do not free vport if register_netdevice() is failed. [Linux 4.14.129]

net: openvswitch: do not free vport if register_netdevice() is failed. [Linux 4.14.129]

This Linux kernel change "net: openvswitch: do not free vport if register_netdevice() is failed" is included in the Linux 4.14.129 release. This change is authored by Taehee Yoo <ap420073 [at] gmail.com> on Sun Jun 9 23:26:21 2019 +0900. The commit for this change in Linux stable tree is 60086c3 (patch) which is from upstream commit 309b669. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream 309b669.

net: openvswitch: do not free vport if register_netdevice() is failed.

[ Upstream commit 309b66970ee2abf721ecd0876a48940fa0b99a35 ]

In order to create an internal vport, internal_dev_create() is used and
that calls register_netdevice() internally.
If register_netdevice() fails, it calls dev->priv_destructor() to free
private data of netdev. actually, a private data of this is a vport.

Hence internal_dev_create() should not free and use a vport after failure
of register_netdevice().

Test command
    ovs-dpctl add-dp bonding_masters

Splat looks like:
[ 1035.667767] kasan: GPF could be caused by NULL-ptr deref or user memory access
[ 1035.675958] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 1035.676916] CPU: 1 PID: 1028 Comm: ovs-vswitchd Tainted: G    B             5.2.0-rc3+ #240
[ 1035.676916] RIP: 0010:internal_dev_create+0x2e5/0x4e0 [openvswitch]
[ 1035.676916] Code: 48 c1 ea 03 80 3c 02 00 0f 85 9f 01 00 00 4c 8b 23 48 b8 00 00 00 00 00 fc ff df 49 8d bc 24 60 05 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 86 01 00 00 49 8b bc 24 60 05 00 00 e8 e4 68 f4
[ 1035.713720] RSP: 0018:ffff88810dcb7578 EFLAGS: 00010206
[ 1035.713720] RAX: dffffc0000000000 RBX: ffff88810d13fe08 RCX: ffffffff84297704
[ 1035.713720] RDX: 00000000000000ac RSI: 0000000000000000 RDI: 0000000000000560
[ 1035.713720] RBP: 00000000ffffffef R08: fffffbfff0d3b881 R09: fffffbfff0d3b881
[ 1035.713720] R10: 0000000000000001 R11: fffffbfff0d3b880 R12: 0000000000000000
[ 1035.768776] R13: 0000607ee460b900 R14: ffff88810dcb7690 R15: ffff88810dcb7698
[ 1035.777709] FS:  00007f02095fc980(0000) GS:ffff88811b400000(0000) knlGS:0000000000000000
[ 1035.777709] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1035.777709] CR2: 00007ffdf01d2f28 CR3: 0000000108258000 CR4: 00000000001006e0
[ 1035.777709] Call Trace:
[ 1035.777709]  ovs_vport_add+0x267/0x4f0 [openvswitch]
[ 1035.777709]  new_vport+0x15/0x1e0 [openvswitch]
[ 1035.777709]  ovs_vport_cmd_new+0x567/0xd10 [openvswitch]
[ 1035.777709]  ? ovs_dp_cmd_dump+0x490/0x490 [openvswitch]
[ 1035.777709]  ? __kmalloc+0x131/0x2e0
[ 1035.777709]  ? genl_family_rcv_msg+0xa54/0x1030
[ 1035.777709]  genl_family_rcv_msg+0x63a/0x1030
[ 1035.777709]  ? genl_unregister_family+0x630/0x630
[ 1035.841681]  ? debug_show_all_locks+0x2d0/0x2d0
[ ... ]

Fixes: cf124db566e6 ("net: Fix inconsistent teardown and release of private netdev state.")
Signed-off-by: Taehee Yoo <[email protected]>
Reviewed-by: Greg Rose <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

There are 18 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 net/openvswitch/vport-internal_dev.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 04a3128..b9377af 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -176,7 +176,9 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
 {
    struct vport *vport;
    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)) {
@@ -184,8 +186,9 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
        goto error;
    }

-   vport->dev = alloc_netdev(sizeof(struct internal_dev),
-                 parms->name, NET_NAME_USER, do_setup);
+   dev = alloc_netdev(sizeof(struct internal_dev),
+              parms->name, NET_NAME_USER, do_setup);
+   vport->dev = dev;
    if (!vport->dev) {
        err = -ENOMEM;
        goto error_free_vport;
@@ -207,8 +210,10 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)

    rtnl_lock();
    err = register_netdevice(vport->dev);
-   if (err)
+   if (err) {
+       free_vport = false;
        goto error_unlock;
+   }

    dev_set_promiscuity(vport->dev, 1);
    rtnl_unlock();
@@ -218,11 +223,12 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)

 error_unlock:
    rtnl_unlock();
-   free_percpu(vport->dev->tstats);
+   free_percpu(dev->tstats);
 error_free_netdev:
-   free_netdev(vport->dev);
+   free_netdev(dev);
 error_free_vport:
-   ovs_vport_free(vport);
+   if (free_vport)
+       ovs_vport_free(vport);
 error:
    return ERR_PTR(err);
 }

Leave a Reply

Your email address will not be published. Required fields are marked *