[rtnl] b1585bdfb2: kernel_BUG_at_net/core/dev.c

FYI, we noticed the following commit (built with gcc-7):

commit: b1585bdfb20476e31f11c3fd44faac770325b8c1 ("rtnl: device allocation/free via rtnl_link_ops")
url: https://github.com/0day-ci/linux/commits/Denis-Drozdov/rtnl-device-allocation-free-via-rtnl_link_ops/20180102-043933


in testcase: trinity
with following parameters:

	runtime: 300s

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/


on test machine: qemu-system-x86_64 -enable-kvm -m 512M

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+------------------------------------------+------------+------------+
|                                          | ead68f2161 | b1585bdfb2 |
+------------------------------------------+------------+------------+
| boot_successes                           | 260        | 2          |
| boot_failures                            | 4          | 13         |
| BUG:kernel_hang_in_boot_stage            | 3          |            |
| BUG:kernel_hang_in_test_stage            | 1          |            |
| kernel_BUG_at_net/core/dev.c             | 0          | 13         |
| invalid_opcode:#[##]                     | 0          | 13         |
| RIP:free_netdev                          | 0          | 13         |
| general_protection_fault:#[##]           | 0          | 9          |
| RIP:put_page                             | 0          | 9          |
| Kernel_panic-not_syncing:Fatal_exception | 0          | 13         |
+------------------------------------------+------------+------------+



[   35.401334] kernel BUG at net/core/dev.c:8231!
[   35.402342] invalid opcode: 0000 [#1] SMP
[   35.403055] Modules linked in:
[   35.403636] CPU: 0 PID: 134 Comm: kworker/u2:2 Not tainted 4.15.0-rc4-00203-gb1585bd #74
[   35.404979] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[   35.407023] Workqueue: netns cleanup_net
[   35.407760] RIP: 0010:free_netdev+0xde/0xf7
[   35.408491] RSP: 0018:ffffa49a00af7d48 EFLAGS: 00010202
[   35.409356] RAX: 0000000000000004 RBX: ffff8b8ca7856000 RCX: 0000000000000001
[   35.410473] RDX: 0000000000000001 RSI: ffffffff8e0e7e00 RDI: 0000000000000000
[   35.411597] RBP: ffff8b8ca7855fd8 R08: 0000000000000080 R09: 0000000000000004
[   35.412722] R10: 00000000ffffffd2 R11: 0000000000000000 R12: ffff8b8ca7856060
[   35.413847] R13: 00000000fffefd9a R14: 00000000fffefd9a R15: dead000000000100
[   35.414976] FS:  0000000000000000(0000) GS:ffff8b8cbf400000(0000) knlGS:0000000000000000
[   35.416305] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   35.417242] CR2: 000000000325de08 CR3: 0000000034559000 CR4: 00000000000006f0
[   35.418347] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   35.419455] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   35.420576] Call Trace:
[   35.421081]  netdev_run_todo+0x247/0x290
[   35.421770]  ip6_tnl_exit_batch_net+0x150/0x15f
[   35.422549]  cleanup_net+0x19b/0x234
[   35.423195]  process_one_work+0x2b3/0x4c3
[   35.423894]  ? worker_thread+0x1e0/0x25b
[   35.424574]  ? rescuer_thread+0x27a/0x27a
[   35.425274]  worker_thread+0x1a3/0x25b
[   35.425940]  ? rescuer_thread+0x27a/0x27a
[   35.426631]  kthread+0xf5/0xfa
[   35.427199]  ? kthread_create_worker_on_cpu+0x43/0x43
[   35.428056]  ? call_usermodehelper_exec_async+0x118/0x11f
[   35.428954]  ? do_group_exit+0xb2/0xb2
[   35.429618]  ret_from_fork+0x24/0x30
[   35.430266] Code: 00 00 00 74 0c 48 c7 c7 d0 a6 4b 8e e8 4e 5f ff ff 8a 83 80 05 00 00 84 c0 75 0c 48 89 df 5b 5d 41 5c e9 12 ff ff ff 3c 03 74 02 <0f> 0b c6 83 80 05 00 00 04 48 8d bb b0 05 00 00 5b 5d 41 5c e9 
[   35.433215] RIP: free_netdev+0xde/0xf7 RSP: ffffa49a00af7d48
[   35.434198] ---[ end trace def93e3513cf4702 ]---


To reproduce:

        git clone https://github.com/intel/lkp-tests.git
        cd lkp-tests
        bin/lkp qemu -k <bzImage> job-script  # job-script is attached in this email



Thanks,
lkp

IP link was broken due to the changes in IPoIB for the rdma_netdev
support after commit cd565b4b51e5
("IB/IPoIB: Support acceleration options callbacks").

This patchset restores IPoIB pkey creation and removal using rtnetlink.
The first patch introduces changes in the rtnetlink code in order to allow
IPOIB allocate and free the netdevice.

The second patch establishes appropriate rtnetlink callbacks for IPoIB
device and restores IPoIB netlink support

Changes since v1:
- Fixed double free_netdev calls in ops->free_link in netdev_run_todo 
- Removed priv_size from ipoib_link_ops as not required anymore.


Denis Drozdov (2):
  rtnl: device allocation/free via rtnl_link_ops
  IB/ipoib: Fix netlink support in IPoIB

 drivers/infiniband/ulp/ipoib/ipoib.h         |  2 ++
 drivers/infiniband/ulp/ipoib/ipoib_main.c    | 23 ++++++++--------
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c | 39 +++++++++++++++++++++++++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c    | 20 ++++----------
 include/net/rtnetlink.h                      |  4 +++
 net/core/dev.c                               |  2 ++
 net/core/rtnetlink.c                         | 29 ++++++++++++++++-----
 7 files changed, 82 insertions(+), 37 deletions(-)

-- 
1.8.3.1

IPoIB netlink support was broken by commit cd565b4b51e5
("IB/IPoIB: Support acceleration options callbacks"),
that added flow which allocates netdev rdma structures
after netlink object is already created. Such situation leads
to crash in __ipoib_device_add, once trying to reuse netlink
device.
This commit restores the netlink support.

Fixes: cd565b4b51e5 ("IB/IPoIB: Support acceleration options callbacks")
Signed-off-by: Denis Drozdov <redacted>
Reviewed-by: Erez Shitrit <redacted>
Reviewed-by: Leon Romanovsky <redacted>
Reviewed-by: Saeed Mahameed <redacted>
---
 drivers/infiniband/ulp/ipoib/ipoib.h         |  2 ++
 drivers/infiniband/ulp/ipoib/ipoib_main.c    | 23 ++++++++--------
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c | 39 +++++++++++++++++++++++++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c    | 20 ++++----------
 4 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 8033a00..aa7a02f 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h

@@ -607,6 +607,8 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
 void ipoib_set_ethtool_ops(struct net_device *dev);
 void ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca);
 
+void ipoib_free_rdma_netdev(struct net_device *dev);
+
 #define IPOIB_FLAGS_RC		0x80
 #define IPOIB_FLAGS_UC		0x40
 

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 8880351d..6e7548e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c

@@ -2022,6 +2022,14 @@ struct ipoib_dev_priv *ipoib_intf_alloc(struct ib_device *hca, u8 port,
 	return NULL;
 }
 
+void ipoib_free_rdma_netdev(struct net_device *dev)
+{
+	struct rdma_netdev *rn = netdev_priv(dev);
+
+	rn->free_rdma_netdev(dev);
+	kfree(ipoib_priv(dev));
+}
+
 static ssize_t show_pkey(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {

@@ -2203,7 +2211,6 @@ static struct net_device *ipoib_add_port(const char *format,
 {
 	struct ipoib_dev_priv *priv;
 	struct ib_port_attr attr;
-	struct rdma_netdev *rn;
 	int result = -ENOMEM;
 
 	priv = ipoib_intf_alloc(hca, port, format);

@@ -2303,9 +2310,7 @@ static struct net_device *ipoib_add_port(const char *format,
 	ipoib_dev_cleanup(priv->dev);
 
 device_init_failed:
-	rn = netdev_priv(priv->dev);
-	rn->free_rdma_netdev(priv->dev);
-	kfree(priv);
+	ipoib_free_rdma_netdev(priv->dev);
 
 alloc_mem_failed:
 	return ERR_PTR(result);

@@ -2378,13 +2383,9 @@ static void ipoib_remove_one(struct ib_device *device, void *client_data)
 
 		parent_rn->free_rdma_netdev(priv->dev);
 
-		list_for_each_entry_safe(cpriv, tcpriv, &priv->child_intfs, list) {
-			struct rdma_netdev *child_rn;
-
-			child_rn = netdev_priv(cpriv->dev);
-			child_rn->free_rdma_netdev(cpriv->dev);
-			kfree(cpriv);
-		}
+		list_for_each_entry_safe(cpriv, tcpriv,
+					 &priv->child_intfs, list)
+			ipoib_free_rdma_netdev(cpriv->dev);
 
 		kfree(priv);
 	}

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
index 3e44087..f5cbb7a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c

@@ -93,12 +93,38 @@ static int ipoib_changelink(struct net_device *dev, struct nlattr *tb[],
 	return ret;
 }
 
+static struct net_device *ipoib_alloc_link(struct net *src_net,
+					   const char *dev_name,
+					   struct nlattr *tb[])
+{
+	struct net_device *pdev;
+	struct ipoib_dev_priv *ppriv, *priv;
+
+	if (!tb[IFLA_LINK])
+		return ERR_PTR(-EINVAL);
+
+	ASSERT_RTNL();
+	pdev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+	if (!pdev || pdev->type != ARPHRD_INFINIBAND)
+		return ERR_PTR(-ENODEV);
+
+	ppriv = ipoib_priv(pdev);
+
+	priv = ipoib_intf_alloc(ppriv->ca, ppriv->port, dev_name);
+	if (!priv) {
+		ipoib_warn(ppriv, "failed to allocate pkey device\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return priv->dev;
+}
+
 static int ipoib_new_child_link(struct net *src_net, struct net_device *dev,
 				struct nlattr *tb[], struct nlattr *data[],
 				struct netlink_ext_ack *extack)
 {
 	struct net_device *pdev;
-	struct ipoib_dev_priv *ppriv;
+	struct ipoib_dev_priv *ppriv, *priv;
 	u16 child_pkey;
 	int err;
 

@@ -131,11 +157,15 @@ static int ipoib_new_child_link(struct net *src_net, struct net_device *dev,
 	 */
 	child_pkey |= 0x8000;
 
-	err = __ipoib_vlan_add(ppriv, ipoib_priv(dev),
-			       child_pkey, IPOIB_RTNL_CHILD);
+	down_write(&ppriv->vlan_rwsem);
+
+	priv = ipoib_priv(dev);
+	err = __ipoib_vlan_add(ppriv, priv, child_pkey, IPOIB_RTNL_CHILD);
+	up_write(&ppriv->vlan_rwsem);
 
 	if (!err && data)
 		err = ipoib_changelink(dev, tb, data, extack);
+
 	return err;
 }
 

@@ -163,13 +193,14 @@ static size_t ipoib_get_size(const struct net_device *dev)
 	.kind		= "ipoib",
 	.maxtype	= IFLA_IPOIB_MAX,
 	.policy		= ipoib_policy,
-	.priv_size	= sizeof(struct ipoib_dev_priv),
 	.setup		= ipoib_setup_common,
 	.newlink	= ipoib_new_child_link,
 	.changelink	= ipoib_changelink,
 	.dellink	= ipoib_unregister_child_dev,
 	.get_size	= ipoib_get_size,
 	.fill_info	= ipoib_fill_info,
+	.alloc_link     = ipoib_alloc_link,
+	.free_link	= ipoib_free_rdma_netdev
 };
 
 int __init ipoib_netlink_init(void)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
index 55a9b71..3ebf6de 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c

@@ -162,29 +162,23 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
 		result = -ENOTUNIQ;
 		goto out;
 	}
-
 	list_for_each_entry(tpriv, &ppriv->child_intfs, list) {
 		if (tpriv->pkey == pkey &&
-		    tpriv->child_type == IPOIB_LEGACY_CHILD) {
+		    (tpriv->child_type == IPOIB_LEGACY_CHILD ||
+		     tpriv->child_type == IPOIB_RTNL_CHILD)) {
 			result = -ENOTUNIQ;
 			goto out;
 		}
 	}
 
 	result = __ipoib_vlan_add(ppriv, priv, pkey, IPOIB_LEGACY_CHILD);
-
 out:
 	up_write(&ppriv->vlan_rwsem);
 	rtnl_unlock();
 	mutex_unlock(&ppriv->sysfs_mutex);
 
-	if (result && priv) {
-		struct rdma_netdev *rn;
-
-		rn = netdev_priv(priv->dev);
-		rn->free_rdma_netdev(priv->dev);
-		kfree(priv);
-	}
+	if (result && priv)
+		ipoib_free_rdma_netdev(priv->dev);
 
 	return result;
 }

@@ -235,11 +229,7 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey)
 	mutex_unlock(&ppriv->sysfs_mutex);
 
 	if (dev) {
-		struct rdma_netdev *rn;
-
-		rn = netdev_priv(dev);
-		rn->free_rdma_netdev(priv->dev);
-		kfree(priv);
+		ipoib_free_rdma_netdev(dev);
 		return 0;
 	}
 

-- 
1.8.3.1

From: Denis Drozdov <redacted>
Date: Tue,  9 Jan 2018 23:42:45 +0200

IP link was broken due to the changes in IPoIB for the rdma_netdev
support after commit cd565b4b51e5
("IB/IPoIB: Support acceleration options callbacks").

This patchset restores IPoIB pkey creation and removal using rtnetlink.
The first patch introduces changes in the rtnetlink code in order to allow
IPOIB allocate and free the netdevice.

The second patch establishes appropriate rtnetlink callbacks for IPoIB
device and restores IPoIB netlink support

Changes since v1:
- Fixed double free_netdev calls in ops->free_link in netdev_run_todo 
- Removed priv_size from ipoib_link_ops as not required anymore.

Please fix your control flow so that the existing netlink op can do
the right thing.

We're not going to add a whole new set of netdev ops every time a subsystem
has a weird control flow issue that no other device type has.  Please fix
this inside of the IB stack, thank you.