Re: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte

From: Vinicius Costa Gomes
Date: Tue Sep 03 2019 - 17:26:50 EST


Hi,

Vladimir Oltean <olteanv@xxxxxxxxx> writes:

> Right. And while we're at it, there's still the potential
> division-by-zero problem which I still don't know how to solve without
> implementing a full-blown __ethtool_get_link_ksettings parser that
> checks against all the possible outputs it can have under the "no
> carrier" condition - see "[RFC PATCH 1/1] phylink: Set speed to
> SPEED_UNKNOWN when there is no PHY connected" for details.
> And there's also a third fix to be made: the netdev_dbg should be made
> to print "speed" instead of "ecmd.base.speed".

For the ksettings part I am thinking on adding something like this to
ethtool.c. Do you think anything is missing (apart from the
documentation)?

->

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 95991e43..d37c80b 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -177,6 +177,9 @@ void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
const unsigned long *src);

+u32 ethtool_link_ksettings_to_speed(const struct ethtool_link_ksettings *settings,
+ u32 default_speed);
+
/**
* struct ethtool_ops - optional netdev operations
* @get_drvinfo: Report driver/device information. Should only set the
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6288e69..80e3db3 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -539,6 +539,18 @@ struct ethtool_link_usettings {
} link_modes;
};

+u32 ethtool_link_ksettings_to_speed(const struct ethtool_link_ksettings *settings,
+ u32 default_speed)
+{
+ if (settings->base.speed == SPEED_UNKNOWN)
+ return default_speed;
+
+ if (settings->base.speed == 0)
+ return default_speed;
+
+ return settings->base.speed;
+}
+
/* Internal kernel helper to query a device ethtool_link_settings. */
int __ethtool_get_link_ksettings(struct net_device *dev,
struct ethtool_link_ksettings *link_ksettings)