From patchwork Mon Apr 3 12:04:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sekhar Nori X-Patchwork-Id: 96617 Delivered-To: patch@linaro.org Received: by 10.140.89.233 with SMTP id v96csp65543qgd; Mon, 3 Apr 2017 05:05:02 -0700 (PDT) X-Received: by 10.99.126.13 with SMTP id z13mr15982291pgc.158.1491221102687; Mon, 03 Apr 2017 05:05:02 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 198si14098652pgd.114.2017.04.03.05.05.02; Mon, 03 Apr 2017 05:05:02 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-omap-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@ti.com; spf=pass (google.com: best guess record for domain of linux-omap-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-omap-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751944AbdDCMFB (ORCPT + 4 others); Mon, 3 Apr 2017 08:05:01 -0400 Received: from fllnx209.ext.ti.com ([198.47.19.16]:12866 "EHLO fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751829AbdDCMFA (ORCPT ); Mon, 3 Apr 2017 08:05:00 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v33C4c1N015545; Mon, 3 Apr 2017 07:04:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com; s=ti-com-17Q1; t=1491221078; bh=CistHPI1fmpyKLDXjl8YlnjZXTNmZ2nj5suFSSuMkXo=; h=From:To:CC:Subject:Date; b=KUWxu7BvPGJ/+9KNJ+5+iR8YqDJeC/1CaHe0V4XUmV2UNHJv2aDiQ3f83PaOfMtSe OKyepU3on0IEg9Y4kFtpkbsYx0ZD7UCFSe2Rp9Zkxjm4NrCnCweS5k9GGPc7L4IklX gE92pATXcy0SE2cC9UB3dk30/F/RkrT9lLbbYn9E= Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v33C4Wa4026922; Mon, 3 Apr 2017 07:04:32 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.294.0; Mon, 3 Apr 2017 07:04:32 -0500 Received: from psplinux063.india.ti.com (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v33C4TlQ030764; Mon, 3 Apr 2017 07:04:30 -0500 From: Sekhar Nori To: "David S . Miller" CC: Grygorii Strashko , , Subject: [PATCH] net: ethernet: ti: cpsw: fix race condition during open() Date: Mon, 3 Apr 2017 17:34:28 +0530 Message-ID: <509439ff3e756ea1dade2738e305f08fc7920650.1491220439.git.nsekhar@ti.com> X-Mailer: git-send-email 2.9.0 MIME-Version: 1.0 Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org TI's cpsw driver handles both OF and non-OF case for phy connect. Unfortunately of_phy_connect() returns NULL on error while phy_connect() returns ERR_PTR(). To handle this, cpsw_slave_open() overrides the return value from phy_connect() to make it NULL or error. This leaves a small window, where cpsw_adjust_link() may be invoked for a slave while slave->phy pointer is temporarily set to -ENODEV (or some other error) before it is finally set to NULL. _cpsw_adjust_link() only handles the NULL case, and an oops results when ERR_PTR() is seen by it. Note that cpsw_adjust_link() checks PHY status for each slave whenever it is invoked. It can so happen that even though phy_connect() for a given slave returns error, _cpsw_adjust_link() is still called for that slave because the link status of another slave changed. Fix this by using a temporary pointer to store return value of {of_}phy_connect() and do a one-time write to slave->phy. Reviewed-by: Grygorii Strashko Reported-by: Yan Liu Signed-off-by: Sekhar Nori --- drivers/net/ethernet/ti/cpsw.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 9f3d9c67e3fe..5a4faa4489d0 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -1267,6 +1267,7 @@ static void soft_reset_slave(struct cpsw_slave *slave) static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv) { u32 slave_port; + struct phy_device *phy; struct cpsw_common *cpsw = priv->cpsw; soft_reset_slave(slave); @@ -1300,27 +1301,28 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv) 1 << slave_port, 0, 0, ALE_MCAST_FWD_2); if (slave->data->phy_node) { - slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node, + phy = of_phy_connect(priv->ndev, slave->data->phy_node, &cpsw_adjust_link, 0, slave->data->phy_if); - if (!slave->phy) { + if (!phy) { dev_err(priv->dev, "phy \"%s\" not found on slave %d\n", slave->data->phy_node->full_name, slave->slave_num); return; } } else { - slave->phy = phy_connect(priv->ndev, slave->data->phy_id, + phy = phy_connect(priv->ndev, slave->data->phy_id, &cpsw_adjust_link, slave->data->phy_if); - if (IS_ERR(slave->phy)) { + if (IS_ERR(phy)) { dev_err(priv->dev, "phy \"%s\" not found on slave %d, err %ld\n", slave->data->phy_id, slave->slave_num, - PTR_ERR(slave->phy)); - slave->phy = NULL; + PTR_ERR(phy)); return; } } + slave->phy = phy; + phy_attached_info(slave->phy); phy_start(slave->phy);