hsr: fix error handling routine in hsr_dev_finalize()
authorTaehee Yoo <ap420073@gmail.com>
Sun, 22 Dec 2019 11:26:15 +0000 (11:26 +0000)
committerDavid S. Miller <davem@davemloft.net>
Thu, 26 Dec 2019 00:35:35 +0000 (16:35 -0800)
hsr_dev_finalize() is called to create new hsr interface.
There are some wrong error handling codes.

1. wrong checking return value of debugfs_create_{dir/file}.
These function doesn't return NULL. If error occurs in there,
it returns error pointer.
So, it should check error pointer instead of NULL.

2. It doesn't unregister interface if it fails to setup hsr interface.
If it fails to initialize hsr interface after register_netdevice(),
it should call unregister_netdevice().

3. Ignore failure of creation of debugfs
If creating of debugfs dir and file is failed, creating hsr interface
will be failed. But debugfs doesn't affect actual logic of hsr module.
So, ignoring this is more correct and this behavior is more general.

Fixes: c5a759117210 ("net/hsr: Use list_head (and rcu) instead of array for slave devices.")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/hsr/hsr_debugfs.c
net/hsr/hsr_device.c
net/hsr/hsr_main.h

index 6135706f03d5fe3d25e1d3c50077fcd857640055..6618a9d8e58e1131be8b3a4c002d385c08ddc794 100644 (file)
@@ -77,15 +77,14 @@ static const struct file_operations hsr_fops = {
  * When debugfs is configured this routine sets up the node_table file per
  * hsr device for dumping the node_table entries
  */
-int hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev)
+void hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev)
 {
-       int rc = -1;
        struct dentry *de = NULL;
 
        de = debugfs_create_dir(hsr_dev->name, NULL);
-       if (!de) {
+       if (IS_ERR(de)) {
                pr_err("Cannot create hsr debugfs root\n");
-               return rc;
+               return;
        }
 
        priv->node_tbl_root = de;
@@ -93,13 +92,13 @@ int hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev)
        de = debugfs_create_file("node_table", S_IFREG | 0444,
                                 priv->node_tbl_root, priv,
                                 &hsr_fops);
-       if (!de) {
+       if (IS_ERR(de)) {
                pr_err("Cannot create hsr node_table directory\n");
-               return rc;
+               debugfs_remove(priv->node_tbl_root);
+               priv->node_tbl_root = NULL;
+               return;
        }
        priv->node_tbl_file = de;
-
-       return 0;
 }
 
 /* hsr_debugfs_term - Tear down debugfs intrastructure
index b01e1bae4ddc079d60ac319991fd3867216bff9a..e73549075a03ec126de36660d323d69703fd0034 100644 (file)
@@ -477,30 +477,31 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
 
        res = hsr_add_port(hsr, hsr_dev, HSR_PT_MASTER);
        if (res)
-               goto err_add_port;
+               goto err_add_master;
 
        res = register_netdevice(hsr_dev);
        if (res)
-               goto fail;
+               goto err_unregister;
 
        res = hsr_add_port(hsr, slave[0], HSR_PT_SLAVE_A);
        if (res)
-               goto fail;
+               goto err_add_slaves;
+
        res = hsr_add_port(hsr, slave[1], HSR_PT_SLAVE_B);
        if (res)
-               goto fail;
+               goto err_add_slaves;
 
+       hsr_debugfs_init(hsr, hsr_dev);
        mod_timer(&hsr->prune_timer, jiffies + msecs_to_jiffies(PRUNE_PERIOD));
-       res = hsr_debugfs_init(hsr, hsr_dev);
-       if (res)
-               goto fail;
 
        return 0;
 
-fail:
+err_add_slaves:
+       unregister_netdevice(hsr_dev);
+err_unregister:
        list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)
                hsr_del_port(port);
-err_add_port:
+err_add_master:
        hsr_del_self_node(&hsr->self_node_db);
 
        return res;
index 96fac696a1e1a602fdf46c4df74016b7baf50008..acab9c353a49f516c4a8b182265b9b70e8a5fdd1 100644 (file)
@@ -184,15 +184,12 @@ static inline u16 hsr_get_skb_sequence_nr(struct sk_buff *skb)
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
-int hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev);
+void hsr_debugfs_init(struct hsr_priv *priv, struct net_device *hsr_dev);
 void hsr_debugfs_term(struct hsr_priv *priv);
 #else
-static inline int hsr_debugfs_init(struct hsr_priv *priv,
-                                  struct net_device *hsr_dev)
-{
-       return 0;
-}
-
+static inline void hsr_debugfs_init(struct hsr_priv *priv,
+                                   struct net_device *hsr_dev)
+{}
 static inline void hsr_debugfs_term(struct hsr_priv *priv)
 {}
 #endif