| 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
| ||
|
Message-Id: <20211204201146.4088103-2-vladimir.oltean@nxp.com>
Date: Sat, 4 Dec 2021 22:11:40 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Tobias Waldekranz <tobias@...dekranz.com>,
DENG Qingfang <dqfext@...il.com>,
Alvin Šipraga <alsi@...g-olufsen.dk>,
Kurt Kanzenbach <kurt@...utronix.de>,
Hauke Mehrtens <hauke@...ke-m.de>,
Woojung Huh <woojung.huh@...rochip.com>,
UNGLinuxDriver@...rochip.com, Sean Wang <sean.wang@...iatek.com>,
Landen Chao <Landen.Chao@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Claudiu Manoil <claudiu.manoil@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Linus Walleij <linus.walleij@...aro.org>,
George McCollister <george.mccollister@...il.com>
Subject: [PATCH v2 net-next 1/7] net: dsa: make dp->bridge_num one-based
I have seen too many bugs already due to the fact that we must encode an
invalid dp->bridge_num as a negative value, because the natural tendency
is to check that invalid value using (!dp->bridge_num). Latest example
can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
getting cleared after ports leaving the bridge").
Convert the existing users to assume that dp->bridge_num == 0 is the
encoding for invalid, and valid bridge numbers start from 1.
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk>
---
v1->v2: none
drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++------
include/linux/dsa/8021q.h | 6 +++---
include/net/dsa.h | 6 +++---
net/dsa/dsa2.c | 24 ++++++++++++------------
net/dsa/dsa_priv.h | 5 +++--
net/dsa/port.c | 11 ++++++-----
net/dsa/tag_8021q.c | 12 +++++++-----
net/dsa/tag_dsa.c | 2 +-
8 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f00cbf5753b9..de3401b2c86c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1250,10 +1250,10 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
/* dev is a virtual bridge */
} else {
list_for_each_entry(dp, &dst->ports, list) {
- if (dp->bridge_num < 0)
+ if (!dp->bridge_num)
continue;
- if (dp->bridge_num + 1 + dst->last_switch != dev)
+ if (dp->bridge_num + dst->last_switch != dev)
continue;
br = dp->bridge_dev;
@@ -2527,9 +2527,9 @@ static void mv88e6xxx_crosschip_bridge_leave(struct dsa_switch *ds,
* physical switches, so start from beyond that range.
*/
static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
- int bridge_num)
+ unsigned int bridge_num)
{
- u8 dev = bridge_num + ds->dst->last_switch + 1;
+ u8 dev = bridge_num + ds->dst->last_switch;
struct mv88e6xxx_chip *chip = ds->priv;
int err;
@@ -2542,14 +2542,14 @@ static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
static int mv88e6xxx_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
return mv88e6xxx_map_virtual_bridge_to_pvt(ds, bridge_num);
}
static void mv88e6xxx_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
int err;
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index 254b165f2b44..0af4371fbebb 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -38,13 +38,13 @@ void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num);
+ unsigned int bridge_num);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num);
+ unsigned int bridge_num);
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num);
u16 dsa_tag_8021q_tx_vid(const struct dsa_port *dp);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8ca9d50cbbc2..a23cfbaa09d6 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -257,7 +257,7 @@ struct dsa_port {
bool learning;
u8 stp_state;
struct net_device *bridge_dev;
- int bridge_num;
+ unsigned int bridge_num;
struct devlink_port devlink_port;
bool devlink_port_setup;
struct phylink *pl;
@@ -754,11 +754,11 @@ struct dsa_switch_ops {
/* Called right after .port_bridge_join() */
int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port,
struct net_device *bridge,
- int bridge_num);
+ unsigned int bridge_num);
/* Called right before .port_bridge_leave() */
void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port,
struct net_device *bridge,
- int bridge_num);
+ unsigned int bridge_num);
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
void (*port_fast_age)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 826957b6442b..9606e56710a5 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -141,23 +141,23 @@ static int dsa_bridge_num_find(const struct net_device *bridge_dev)
*/
list_for_each_entry(dst, &dsa_tree_list, list)
list_for_each_entry(dp, &dst->ports, list)
- if (dp->bridge_dev == bridge_dev &&
- dp->bridge_num != -1)
+ if (dp->bridge_dev == bridge_dev && dp->bridge_num)
return dp->bridge_num;
- return -1;
+ return 0;
}
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
{
- int bridge_num = dsa_bridge_num_find(bridge_dev);
+ unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
- if (bridge_num < 0) {
+ if (!bridge_num) {
/* First port that offloads TX forwarding for this bridge */
- bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
- DSA_MAX_NUM_OFFLOADING_BRIDGES);
+ bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
+ DSA_MAX_NUM_OFFLOADING_BRIDGES,
+ 1);
if (bridge_num >= max)
- return -1;
+ return 0;
set_bit(bridge_num, &dsa_fwd_offloading_bridges);
}
@@ -165,12 +165,13 @@ int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
return bridge_num;
}
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num)
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+ unsigned int bridge_num)
{
/* Check if the bridge is still in use, otherwise it is time
* to clean it up so we can reuse this bridge_num later.
*/
- if (dsa_bridge_num_find(bridge_dev) < 0)
+ if (!dsa_bridge_num_find(bridge_dev))
clear_bit(bridge_num, &dsa_fwd_offloading_bridges);
}
@@ -1184,7 +1185,6 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
dp->ds = ds;
dp->index = index;
- dp->bridge_num = -1;
INIT_LIST_HEAD(&dp->list);
list_add_tail(&dp->list, &dst->ports);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 3fb2c37c9b88..70c4a5b36a8b 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -546,8 +546,9 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
struct net_device *master,
const struct dsa_device_ops *tag_ops,
const struct dsa_device_ops *old_tag_ops);
-int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
-void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num);
+unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max);
+void dsa_bridge_num_put(const struct net_device *bridge_dev,
+ unsigned int bridge_num);
/* tag_8021q.c */
int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6d5ebe61280b..9a77bd1373e2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -273,14 +273,14 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp,
struct net_device *bridge_dev)
{
- int bridge_num = dp->bridge_num;
+ unsigned int bridge_num = dp->bridge_num;
struct dsa_switch *ds = dp->ds;
/* No bridge TX forwarding offload => do nothing */
- if (!ds->ops->port_bridge_tx_fwd_unoffload || dp->bridge_num == -1)
+ if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num)
return;
- dp->bridge_num = -1;
+ dp->bridge_num = 0;
dsa_bridge_num_put(bridge_dev, bridge_num);
@@ -295,14 +295,15 @@ static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp,
struct net_device *bridge_dev)
{
struct dsa_switch *ds = dp->ds;
- int bridge_num, err;
+ unsigned int bridge_num;
+ int err;
if (!ds->ops->port_bridge_tx_fwd_offload)
return false;
bridge_num = dsa_bridge_num_get(bridge_dev,
ds->num_fwd_offloading_bridges);
- if (bridge_num < 0)
+ if (!bridge_num)
return false;
dp->bridge_num = bridge_num;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 72cac2c0af7b..df59f16436a5 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -67,10 +67,12 @@
#define DSA_8021Q_PORT(x) (((x) << DSA_8021Q_PORT_SHIFT) & \
DSA_8021Q_PORT_MASK)
-u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num)
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num)
{
- /* The VBID value of 0 is reserved for precise TX */
- return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1);
+ /* The VBID value of 0 is reserved for precise TX, but it is also
+ * reserved/invalid for the bridge_num, so all is well.
+ */
+ return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num);
}
EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
@@ -409,7 +411,7 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
@@ -420,7 +422,7 @@ EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
struct net_device *br,
- int bridge_num)
+ unsigned int bridge_num)
{
u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index b3da4b2ea11c..a7d70ae7cc97 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -140,7 +140,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
* packets on behalf of a virtual switch device with an index
* past the physical switches.
*/
- tag_dev = dst->last_switch + 1 + dp->bridge_num;
+ tag_dev = dst->last_switch + dp->bridge_num;
tag_port = 0;
} else {
cmd = DSA_CMD_FROM_CPU;
--
2.25.1
Powered by blists - more mailing lists