netdev - [PATCH net-next 09/11] net: dsa: felix: remove dead code in felix_setup_mmio

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Feb 2022 16:30:12 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Xiaoliang Yang <xiaoliang.yang_1@....com>,
        Yangbo Lu <yangbo.lu@....com>, Michael Walle <michael@...le.cc>
Subject: [PATCH net-next 09/11] net: dsa: felix: remove dead code in felix_setup_mmio_filtering()

There has been some controversy related to the sanity check that a CPU
port exists, and commit e8b1d7698038 ("net: dsa: felix: Fix memory leak
in felix_setup_mmio_filtering") even "corrected" an apparent memory leak
as static analysis tools see it.

However, the check is completely dead code, since the earliest point at
which felix_setup_mmio_filtering() can be called is:

felix_pci_probe
-> dsa_register_switch
   -> dsa_switch_probe
      -> dsa_tree_setup
         -> dsa_tree_setup_cpu_ports
            -> dsa_tree_setup_default_cpu
               -> contains the "DSA: tree %d has no CPU port\n" check
         -> dsa_tree_setup_master
            -> dsa_master_setup
               -> sysfs_create_group(&dev->dev.kobj, &dsa_group);
                  -> makes tagging_store() callable
                     -> dsa_tree_change_tag_proto
                        -> dsa_tree_notify
                           -> dsa_switch_event
                              -> dsa_switch_change_tag_proto
                                 -> ds->ops->change_tag_protocol
                                    -> felix_change_tag_protocol
                                       -> felix_set_tag_protocol
                                          -> felix_setup_tag_8021q
                                             -> felix_setup_mmio_filtering
                                                -> breaks at first CPU port

So probing would have failed earlier if there wasn't any CPU port
defined.

To avoid all confusion, delete the dead code and replace it with a
comment.

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 drivers/net/dsa/ocelot/felix.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 549c41a0ebe0..f0ac26ac1585 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -296,11 +296,9 @@ static int felix_setup_mmio_filtering(struct felix *felix)
 		break;
 	}
 
-	if (cpu < 0) {
-		kfree(tagging_rule);
-		kfree(redirect_rule);
-		return -EINVAL;
-	}
+	/* We are sure that "cpu" was found, otherwise
+	 * dsa_tree_setup_default_cpu() would have failed earlier.
+	 */
 
 	tagging_rule->key_type = OCELOT_VCAP_KEY_ETYPE;
 	*(__be16 *)tagging_rule->key.etype.etype.value = htons(ETH_P_1588);
-- 
2.25.1