[PATCH net v2 0/2] net: macb: fix the link speed the taprio setup reads

[PATCH net v2 0/2] net: macb: fix the link speed the taprio setup reads

[Aleksei Sviridkin]

Two small fixes in macb_taprio_setup_replace(), both in how it obtains
the link speed it scales the schedule with.

The first: it hands phylink_ethtool_ksettings_get() a stack variable
it never zeroed, while phylink fills only what the link mode provides
and even reads one field back from the caller. The second: the speed
check is written as "<= 0" on a u32, so SPEED_UNKNOWN passes it and
turns into a 1 ns hardware limit that every entry then exceeds.

Compile-tested against net; the driver has no test surface, and no
macb board here.

---
v2: both commit messages rewritten to name the uninitialised-memory
    case first and the link-down case as the one that happens; the
    grown declaration keeps reverse xmas tree order; the error text is
    "Invalid speed %d, link-down?" (Théo Lebrun). ethtool_validate_speed()
    was considered for the check and does not fit: it accepts
    SPEED_UNKNOWN by definition, which is exactly the value patch 2
    rejects (Andrew Lunn).
    https://lore.kernel.org/netdev/20260902080528.2211468-1-f@lex.la/


Aleksei Sviridkin (2):
  net: macb: zero the link settings taprio reads back
  net: macb: reject an unknown link speed in the taprio setup

 drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH net v2 1/2] net: macb: zero the link settings taprio reads back

[Aleksei Sviridkin]

macb_taprio_setup_replace() calls phylink_ethtool_ksettings_get() with
an uninitialised kset, and kset is not only an out-parameter. On a
fixed link, or an in-band link with no PHY, phylink writes speed and
duplex only if kset->base.rate_matching already reads RATE_MATCH_NONE,
a field it never writes itself; in PHY mode before the PHY is attached
it writes port and supported and nothing more. Either way the speed
read back afterwards can be stack garbage. The ethtool core zeroes the
structure on every path into the op, which is why its callers never
see this; taprio is the only in-kernel caller passing its own variable.

Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
v2: message rewritten, declaration in reverse xmas tree order (Théo
    Lebrun).

 drivers/net/ethernet/cadence/macb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 76ee4f506033..61838084989a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4295,9 +4295,9 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
 	u64 total_on_time = 0, start_time_sec = 0, start_time = conf->base_time;
 	u32 configured_queues = 0, speed = 0, start_time_nsec;
 	struct macb_queue_enst_config *enst_queue;
-	struct tc_taprio_sched_entry *entry;
+	struct ethtool_link_ksettings kset = {};
 	struct macb *bp = netdev_priv(netdev);
-	struct ethtool_link_ksettings kset;
+	struct tc_taprio_sched_entry *entry;
 	struct macb_queue *queue;
 	u32 queue_mask;
 	u8 queue_id;
-- 
2.53.0

[PATCH net v2 2/2] net: macb: reject an unknown link speed in the taprio setup

[Aleksei Sviridkin]

speed is a u32, so SPEED_UNKNOWN arrives as 0xffffffff and passes the
"speed <= 0" check, which only ever catches zero. That is what an
autonegotiating link reports while it is down: the limit derived from
the speed collapses to a nanosecond at most and the first entry fails
with a misleading "exceeds hardware limit". Zero stays covered, it is
what an interface that was never opened reports, and
enst_max_hw_interval() divides by it. Say which case it was in the
error.

Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
ethtool_validate_speed() accepts SPEED_UNKNOWN (and zero), so it cannot
replace this check.

v2: error text and message per Théo Lebrun.

 drivers/net/ethernet/cadence/macb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 61838084989a..202bc688978c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4324,8 +4324,8 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
 	}
 
 	speed = kset.base.speed;
-	if (unlikely(speed <= 0)) {
-		netdev_err(netdev, "Invalid speed: %d\n", speed);
+	if (unlikely(speed == SPEED_UNKNOWN || !speed)) {
+		netdev_err(netdev, "Invalid speed %d, link-down?\n", speed);
 		return -EINVAL;
 	}
 
-- 
2.53.0