From patchwork Wed Aug 28 14:58:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 1154536 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="DcsMq/re"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 46JTQ32gjjz9sDQ for ; Thu, 29 Aug 2019 00:58:19 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726504AbfH1O6S (ORCPT ); Wed, 28 Aug 2019 10:58:18 -0400 Received: from mail-wr1-f68.google.com ([209.85.221.68]:37703 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726415AbfH1O6R (ORCPT ); Wed, 28 Aug 2019 10:58:17 -0400 Received: by mail-wr1-f68.google.com with SMTP id z11so138124wrt.4 for ; Wed, 28 Aug 2019 07:58:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=nA1t6hIpZCFd0iG4w43rTL1Fyg561GbLFe0cwUNcQfY=; b=DcsMq/re6gIrVuQl5OTZyJ3b6vAQfIq+FybfzuwcE0aktn6HW2pJBJ59Eh4zlXLSWS 5z9tYyAtBSVyMCWstk5tXZPPpILsSzgn5BI6ZOw/FfRbcQc4Ec6nuG6TShfxjIFA6swp StkfI+I9VrbW7ke9XpIPcnk9FwhKLivoN3ljNooJGNeE3Hj+E7wcYGA3OCBW5yqDpZz3 ir1Sm4eN5G7iMZQgiJ95Yu0I+b+1IIwJ4LjA4nAfQwz3Omad9qbmlWeZcl8lBU2hX6jP Ni+6EP4aL5J89JQhfxIXGQYEEVP12aI7d6GmlrdRt0fUElBASFQh6rBq9dLAFmPChwTq MXnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=nA1t6hIpZCFd0iG4w43rTL1Fyg561GbLFe0cwUNcQfY=; b=TCZ8kENYb2nyU87yrOH39fJLrkV/mLfE8ZFjVmIzRcFw+MqeToIqtV9vyZxsBURCms xQGcv5eFgozudymR/eO9EvEOgCECPvzM6BXHWNlG/b6RzdEhPBdqYk1/9yo+mhX84mRk crhdq1sGf2mrsUZdljQk7Pm+3w/lE5R8S8B6EABnm9nnWGEViHYFon844y/AzE68nI9G d0OfZdZDxsg3YZolYZvRc42H/E2FQRfXT7eeIOVaDkRbo24nIa6BKUjunqQhlXng6yua BkX7UkVb2jz+zsDaGQciWEKfYb8nH9AbnQtky1EqjBloFMC2W8LxIxyQL9A5JWZZMW7r IKew== X-Gm-Message-State: APjAAAXAIw1IbF/d/FW4awpF+OAZTl2GREEOdMp/YFTKR6C8Y87SCxI9 nbippbW7w4trgFpWw9mH+Yc= X-Google-Smtp-Source: APXvYqxbayafR+/wiB/wFxOPH8pKJD9yn+6EAYWiIkqnwQ17lv066TUYetubQQrAaYd4IRRzCp3Uig== X-Received: by 2002:adf:e444:: with SMTP id t4mr4996086wrm.262.1567004294074; Wed, 28 Aug 2019 07:58:14 -0700 (PDT) Received: from localhost.localdomain ([86.126.25.232]) by smtp.gmail.com with ESMTPSA id o2sm3284190wmh.9.2019.08.28.07.58.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Aug 2019 07:58:13 -0700 (PDT) From: Vladimir Oltean To: linux@armlinux.org.uk, andrew@lunn.ch, f.fainelli@gmail.com, asolokha@kb.kras.ru Cc: netdev@vger.kernel.org, Vladimir Oltean , Russell King Subject: [RFC PATCH 1/1] phylink: Set speed to SPEED_UNKNOWN when there is no PHY connected Date: Wed, 28 Aug 2019 17:58:02 +0300 Message-Id: <20190828145802.3609-2-olteanv@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190828145802.3609-1-olteanv@gmail.com> References: <20190828145802.3609-1-olteanv@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org phylink_ethtool_ksettings_get can be called while the interface may not even be up, which should not be a problem. But there are drivers (e.g. gianfar) which connect to the PHY in .ndo_open and disconnect in .ndo_close. While odd, to my knowledge this is again not illegal and there may be more that do the same. But PHYLINK for example has this check in phylink_ethtool_ksettings_get: if (pl->phydev) { phy_ethtool_ksettings_get(pl->phydev, kset); } else { kset->base.port = pl->link_port; } So it will not populate kset->base.speed if there is no PHY connected. The speed will be 0, by way of a previous memset. Not SPEED_UNKNOWN. It is arguable whether that is legal or not. include/uapi/linux/ethtool.h says: All values 0 to INT_MAX are legal. By that measure it may be. But it sure would make users of the __ethtool_get_link_ksettings API need make more complicated checks (against -1, against 0, 1, etc). So far the kernel community has been ok with just checking for SPEED_UNKNOWN. Take net/sched/sch_taprio.c for example. The check in taprio_set_picos_per_byte is currently not robust enough and will trigger this division by zero, due to PHYLINK not setting SPEED_UNKNOWN: if (!__ethtool_get_link_ksettings(dev, &ecmd) && ecmd.base.speed != SPEED_UNKNOWN) picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8, ecmd.base.speed * 1000 * 1000); [ 27.109992] Division by zero in kernel. [ 27.113842] CPU: 1 PID: 198 Comm: tc Not tainted 5.3.0-rc5-01246-gc4006b8c2637-dirty #212 [ 27.121974] Hardware name: Freescale LS1021A [ 27.126234] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 27.133938] [] (show_stack) from [] (dump_stack+0xb0/0xc4) [ 27.141124] [] (dump_stack) from [] (Ldiv0_64+0x8/0x18) [ 27.148052] [] (Ldiv0_64) from [] (div64_u64+0xcc/0xf0) [ 27.154978] [] (div64_u64) from [] (div64_s64+0x4c/0x68) [ 27.161993] [] (div64_s64) from [] (taprio_set_picos_per_byte+0xe8/0xf4) [ 27.170388] [] (taprio_set_picos_per_byte) from [] (taprio_change+0x668/0xcec) [ 27.179302] [] (taprio_change) from [] (qdisc_create+0x1fc/0x4f4) [ 27.187091] [] (qdisc_create) from [] (tc_modify_qdisc+0x1ac/0x6f8) [ 27.195055] [] (tc_modify_qdisc) from [] (rtnetlink_rcv_msg+0x268/0x2dc) [ 27.203449] [] (rtnetlink_rcv_msg) from [] (netlink_rcv_skb+0xe0/0x114) [ 27.211756] [] (netlink_rcv_skb) from [] (netlink_unicast+0x1b4/0x22c) [ 27.219977] [] (netlink_unicast) from [] (netlink_sendmsg+0x284/0x340) [ 27.228198] [] (netlink_sendmsg) from [] (sock_sendmsg+0x14/0x24) [ 27.235988] [] (sock_sendmsg) from [] (___sys_sendmsg+0x214/0x228) [ 27.243863] [] (___sys_sendmsg) from [] (__sys_sendmsg+0x50/0x8c) [ 27.251652] [] (__sys_sendmsg) from [] (ret_fast_syscall+0x0/0x54) [ 27.259524] Exception stack(0xe8045fa8 to 0xe8045ff0) [ 27.264546] 5fa0: b6f608c8 000000f8 00000003 bed7e2f0 00000000 00000000 [ 27.272681] 5fc0: b6f608c8 000000f8 004ce54c 00000128 5d3ce8c7 00000000 00000026 00505c9c [ 27.280812] 5fe0: 00000070 bed7e298 004ddd64 b6dd1e64 Cc: Russell King Fixes: 9525ae83959b ("phylink: add phylink infrastructure") Signed-off-by: Vladimir Oltean --- drivers/net/phy/phylink.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index a45c5de96ab1..3522eaf3e80c 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -1112,6 +1112,7 @@ int phylink_ethtool_ksettings_get(struct phylink *pl, if (pl->phydev) { phy_ethtool_ksettings_get(pl->phydev, kset); } else { + kset->base.speed = SPEED_UNKNOWN; kset->base.port = pl->link_port; }