netdev - Re: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per

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]

Message-ID: <8736hd9ilm.fsf@intel.com>
Date:   Tue, 03 Sep 2019 14:26:45 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Vladimir Oltean <olteanv@...il.com>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte

Hi,

Vladimir Oltean <olteanv@...il.com> 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)