Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices
From: David Miller @ 2018-08-27 22:25 UTC (permalink / raw)
  To: a3at.mail; +Cc: netdev, hkallweit1, nic_swsd
In-Reply-To: <20180826140309.32310-1-a3at.mail@gmail.com>

From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Sun, 26 Aug 2018 17:03:09 +0300

> I have two Ethernet adapters:
>   r8169 0000:03:01.0 eth0: RTL8169sb/8110sb, 00:14:d1:14:2d:49, XID 10000000, IRQ 18
>   r8169 0000:01:00.0 eth0: RTL8168e/8111e, 64:66:b3:11:14:5d, XID 2c200000, IRQ 30
> And after upgrading from linux 4.15 [1] to linux 4.18+ [2] RTL8169sb failed to
> receive any packets. tcpdump shows a lot of checksum mismatch.
> 
>   [1]: a0f79386a4968b4925da6db2d1daffd0605a4402
>   [2]: 0519359784328bfa92bf0931bf0cff3b58c16932 (4.19 merge window opened)
> 
> I started bisecting and the found that [3] breaks it. According to [4]:
>   "For 8110S, 8110SB, and 8110SC series, the initial value of RxConfig
>   needs to be set after the tx/rx is enabled."
> So I moved rtl_init_rxcfg() after enabling tx/rs and now my adapter works
> (RTL8168e works too).
> 
>   [3]: 3559d81e76bfe3803e89f2e04cf6ef7ab4f3aace
>   [4]: e542a2269f232d61270ceddd42b73a4348dee2bb ("r8169: adjust the RxConfig
> settings.")
> 
> Also drop "rx" from rtl_set_rx_tx_config_registers(), since it does nothing
> with it already.
> 
> Fixes: 3559d81e76bfe3803e89f2e04cf6ef7ab4f3aace ("r8169: simplify
> rtl_hw_start_8169")
> 
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
> Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
> ---
> It looks like calling rtl_init_rxcfg() the second time is fine, but I
> can move it into rtl_hw_start_8169())

Heiner, please review.

^ permalink raw reply

* Re: [PATCH] net: dsa: Drop GPIO includes
From: David Miller @ 2018-08-27 22:24 UTC (permalink / raw)
  To: linus.walleij; +Cc: andrew, vivien.didelot, f.fainelli, netdev
In-Reply-To: <20180826222011.19149-1-linus.walleij@linaro.org>

From: Linus Walleij <linus.walleij@linaro.org>
Date: Mon, 27 Aug 2018 00:20:11 +0200

> Commit 52638f71fcff ("dsa: Move gpio reset into switch driver")
> moved the GPIO handling into the switch drivers but forgot
> to remove the GPIO header includes.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied.

^ permalink raw reply

* Re: [patch net 0/2] net: sched: couple of small fixes
From: David Miller @ 2018-08-27 22:17 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: jiri, netdev, jhs, mrv, mlxsw
In-Reply-To: <CAM_iQpVxP5nbCS1MAR8pBbh_biMYt3jCdLH79by-yQeJ2uOQQA@mail.gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 27 Aug 2018 13:44:56 -0700

> On Mon, Aug 27, 2018 at 11:58 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Jiri Pirko (2):
>>   net: sched: fix extack error message when chain is failed to be
>>     created
>>   net: sched: return -ENOENT when trying to remove filter from
>>     non-existent chain
> 
> Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Series applied.

^ permalink raw reply

* Re: [PATCH net] erspan: set erspan_ver to 1 by default when adding an erspan dev
From: David Miller @ 2018-08-27 22:14 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, u9012063
In-Reply-To: <cc2be1deb411c1ca27eeb6a2b3a10617cad7dab0.1535366492.git.lucien.xin@gmail.com>

From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 27 Aug 2018 18:41:32 +0800

> After erspan_ver is introudced, if erspan_ver is not set in iproute, its
> value will be left 0 by default. Since Commit 02f99df1875c ("erspan: fix
> invalid erspan version."), it has broken the traffic due to the version
> check in erspan_xmit if users are not aware of 'erspan_ver' param, like
> using an old version of iproute.
> 
> To fix this compatibility problem, it sets erspan_ver to 1 by default
> when adding an erspan dev in erspan_setup. Note that we can't do it in
> ipgre_netlink_parms, as this function is also used by ipgre_changelink.
> 
> Fixes: 02f99df1875c ("erspan: fix invalid erspan version.")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net] sctp: remove useless start_fail from sctp_ht_iter in proc
From: David Miller @ 2018-08-27 22:14 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <265533f54ceb4684bb8323c9601a743eed409527.1535366418.git.lucien.xin@gmail.com>

From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 27 Aug 2018 18:40:18 +0800

> After changing rhashtable_walk_start to return void, start_fail would
> never be set other value than 0, and the checking for start_fail is
> pointless, so remove it.
> 
> Fixes: 97a6ec4ac021 ("rhashtable: Change rhashtable_walk_start to return void")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net] sctp: hold transport before accessing its asoc in sctp_transport_get_next
From: David Miller @ 2018-08-27 22:13 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <607dd2950d09fc83404d670a73099523087d4963.1535366311.git.lucien.xin@gmail.com>

From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 27 Aug 2018 18:38:31 +0800

> As Marcelo noticed, in sctp_transport_get_next, it is iterating over
> transports but then also accessing the association directly, without
> checking any refcnts before that, which can cause an use-after-free
> Read.
> 
> So fix it by holding transport before accessing the association. With
> that, sctp_transport_hold calls can be removed in the later places.
> 
> Fixes: 626d16f50f39 ("sctp: export some apis or variables for sctp_diag and reuse some for proc")
> Reported-by: syzbot+fe62a0c9aa6a85c6de16@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [V9fs-developer] [PATCH v2 2/2] 9p: Add refcount to p9_req_t
From: Dominique Martinet @ 2018-08-28  1:57 UTC (permalink / raw)
  To: piaojun, Greg Kurz
  Cc: Tomas Bortoli, lucho, Dominique Martinet, ericvh, netdev,
	linux-kernel, syzkaller, v9fs-developer, rminnich, davem
In-Reply-To: <5B84A051.6070903@huawei.com>

piaojun wrote on Tue, Aug 28, 2018:
> > (Jun, I think you'll need to ask again to rename 'req' to 'rreq' if you
> > think it's important -- I think such a rename should go in a separate
> > patch anyway, there's plenty of time until the 4.20 merge window)
> > 
> 
> I still think such a rename is necessary, and as you said, it will be
> better go in another patch.

Tomas can you send a patch for that please?
It's not very interesting, but might as well finish this properly :)

> >> diff --git a/net/9p/client.c b/net/9p/client.c
> >> index 7942c0bfcc5b..c9bb5d41afa4 100644
> >> --- a/net/9p/client.c
> >> +++ b/net/9p/client.c
> >> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
> >>  
> >>  	err = c->trans_mod->request(c, req);
> >>  	if (err < 0) {
> >> +		/* write won't happen */
> >> +		p9_req_put(req);
> >>  		if (err != -ERESTARTSYS && err != -EFAULT)
> >>  			c->status = Disconnected;
> >>  		goto recalc_sigpending;
> > 
> > p9_client_zc_rpc needs the same put if zc_request failed, I'm not sure
> > why it wasn't here in my draft

Ah, I remember a bit better now, this is not as simple as adding the
same check after zc_request because the zc_request embeds the wait
itself to do its own cleanup after the reply came (unpin pages that were
sent to the server).

This brings in an interesting race condition that if the
wait_event_killable() in the zc_request is interrupted, the user data
pages that were pinned get unpinned and could potentially be moved
before the server replies... Even if they're not moved the user would be
told the read/write failed and could reuse the memory that would be
read/written later.
I'm not sure how this part works but it's probably not great.

Greg, do you have an opinion on this?


This is tricky, we cannot even rely on the refcounting for this as the
zc pages are likely user pages, so it'll be bad if we return from the
syscall and the memory gets accessed later.
On the other hand making that wait non-killable isn't a good solution
either, and we cannot use flush for virtio, so I don't have any idea for
this... Any magic virtio "take-back"?



Well, this would be for another patch anyway - for now I'll just do the
p9_req_put if it hasn't been kicked so that means something like the
following diff.. But my test bed is currently down so I'll wait for
tests to push:
-------8<----------------
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 7728b0acde09..36a1401c0722 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -404,6 +404,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 	struct scatterlist *sgs[4];
 	size_t offs;
 	int need_drop = 0;
+	int kicked = 0;
 
 	p9_debug(P9_DEBUG_TRANS, "virtio request\n");
 
@@ -498,6 +499,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 	}
 	virtqueue_kick(chan->vq);
 	spin_unlock_irqrestore(&chan->lock, flags);
+	kicked = 1;
 	p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n");
 	err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
 	/*
@@ -518,6 +520,10 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 	}
 	kvfree(in_pages);
 	kvfree(out_pages);
+	if (!kicked) {
+		/* reply won't come */
+		p9_req_put(req);
+	}
 	return err;
 }
 
-------8<----------------

-- 
Dominique

^ permalink raw reply related

* [PATCH] net: wireless: ath: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-08-28  1:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kalle Valo, David S. Miller, linux-wireless, netdev
In-Reply-To: <20180828015252.28511-1-robh@kernel.org>

In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/net/wireless/ath/ath6kl/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 58fb227a849f..54132af70094 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -710,8 +710,8 @@ static bool check_device_tree(struct ath6kl *ar)
 	for_each_compatible_node(node, NULL, "atheros,ath6kl") {
 		board_id = of_get_property(node, board_id_prop, NULL);
 		if (board_id == NULL) {
-			ath6kl_warn("No \"%s\" property on %s node.\n",
-				    board_id_prop, node->name);
+			ath6kl_warn("No \"%s\" property on %pOFn node.\n",
+				    board_id_prop, node);
 			continue;
 		}
 		snprintf(board_filename, sizeof(board_filename),
-- 
2.17.1

^ permalink raw reply related

* [PATCH] net: phy: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-08-28  1:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: Andrew Lunn, Florian Fainelli, David S. Miller, netdev
In-Reply-To: <20180828015252.28511-1-robh@kernel.org>

In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.

Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/net/phy/mdio-thunder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-thunder.c b/drivers/net/phy/mdio-thunder.c
index 564616968cad..1546f6398831 100644
--- a/drivers/net/phy/mdio-thunder.c
+++ b/drivers/net/phy/mdio-thunder.c
@@ -73,8 +73,8 @@ static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 		err = of_address_to_resource(node, 0, &r);
 		if (err) {
 			dev_err(&pdev->dev,
-				"Couldn't translate address for \"%s\"\n",
-				node->name);
+				"Couldn't translate address for \"%pOFn\"\n",
+				node);
 			break;
 		}
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH] net: ethernet: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-08-28  1:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: David S. Miller, Yisen Zhuang, Salil Mehta, Sebastian Hesselbarth,
	Felix Fietkau, John Crispin, Sean Wang, Nelson Chang,
	Matthias Brugger, Wingman Kwok, Murali Karicheri, netdev
In-Reply-To: <20180828015252.28511-1-robh@kernel.org>

In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: John Crispin <john@phrozen.org>
Cc: Sean Wang <sean.wang@mediatek.com>
Cc: Nelson Chang <nelson.chang@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Wingman Kwok <w-kwok2@ti.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
Cc: netdev@vger.kernel.org

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c  |  4 +-
 .../net/ethernet/hisilicon/hns/hns_dsaf_mac.c |  8 ++--
 drivers/net/ethernet/marvell/mv643xx_eth.c    |  6 +--
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |  2 +-
 drivers/net/ethernet/sun/sunhme.c             |  2 +-
 drivers/net/ethernet/ti/netcp_core.c          | 22 ++++------
 drivers/net/ethernet/ti/netcp_ethss.c         | 42 +++++++++----------
 7 files changed, 40 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index ac2c3f6a12bc..82722d05fedb 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -446,8 +446,8 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 		goto error;
 	}
 
-	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%s@%llx", np->name,
-		(unsigned long long)res.start);
+	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%pOFn@%llx", np,
+		 (unsigned long long)res.start);
 
 	priv->map = of_iomap(np, 0);
 	if (!priv->map) {
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 1c2326bd76e2..6521d8d53745 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -807,8 +807,8 @@ static int hns_mac_get_info(struct hns_mac_cb *mac_cb)
 			 */
 			put_device(&mac_cb->phy_dev->mdio.dev);
 
-			dev_dbg(mac_cb->dev, "mac%d phy_node: %s\n",
-				mac_cb->mac_id, np->name);
+			dev_dbg(mac_cb->dev, "mac%d phy_node: %pOFn\n",
+				mac_cb->mac_id, np);
 		}
 		of_node_put(np);
 
@@ -825,8 +825,8 @@ static int hns_mac_get_info(struct hns_mac_cb *mac_cb)
 			 * if the phy_dev is found
 			 */
 			put_device(&mac_cb->phy_dev->mdio.dev);
-			dev_dbg(mac_cb->dev, "mac%d phy_node: %s\n",
-				mac_cb->mac_id, np->name);
+			dev_dbg(mac_cb->dev, "mac%d phy_node: %pOFn\n",
+				mac_cb->mac_id, np);
 		}
 		of_node_put(np);
 
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 62f204f32316..1e9bcbdc6a90 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2733,17 +2733,17 @@ static int mv643xx_eth_shared_of_add_port(struct platform_device *pdev,
 
 	memset(&res, 0, sizeof(res));
 	if (of_irq_to_resource(pnp, 0, &res) <= 0) {
-		dev_err(&pdev->dev, "missing interrupt on %s\n", pnp->name);
+		dev_err(&pdev->dev, "missing interrupt on %pOFn\n", pnp);
 		return -EINVAL;
 	}
 
 	if (of_property_read_u32(pnp, "reg", &ppd.port_number)) {
-		dev_err(&pdev->dev, "missing reg property on %s\n", pnp->name);
+		dev_err(&pdev->dev, "missing reg property on %pOFn\n", pnp);
 		return -EINVAL;
 	}
 
 	if (ppd.port_number >= 3) {
-		dev_err(&pdev->dev, "invalid reg property on %s\n", pnp->name);
+		dev_err(&pdev->dev, "invalid reg property on %pOFn\n", pnp);
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6e6abdc399de..b44bcfd85b05 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -405,7 +405,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
 	eth->mii_bus->priv = eth;
 	eth->mii_bus->parent = eth->dev;
 
-	snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%s", mii_np->name);
+	snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%pOFn", mii_np);
 	ret = of_mdiobus_register(eth->mii_bus, mii_np);
 
 err_put_node:
diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index 06da2f59fcbf..863fd602fd33 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -2999,7 +2999,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev,
 	/* Now make sure pci_dev cookie is there. */
 #ifdef CONFIG_SPARC
 	dp = pci_device_to_OF_node(pdev);
-	strcpy(prom_name, dp->name);
+	snprintf(prom_name, sizeof(prom_name), "%pOFn", dp);
 #else
 	if (is_quattro_p(pdev))
 		strcpy(prom_name, "SUNW,qfe");
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index a1d335a3c5e4..cb240e0d515a 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -225,17 +225,6 @@ static int emac_arch_get_mac_addr(char *x, void __iomem *efuse_mac, u32 swap)
 	return 0;
 }
 
-static const char *netcp_node_name(struct device_node *node)
-{
-	const char *name;
-
-	if (of_property_read_string(node, "label", &name) < 0)
-		name = node->name;
-	if (!name)
-		name = "unknown";
-	return name;
-}
-
 /* Module management routines */
 static int netcp_register_interface(struct netcp_intf *netcp)
 {
@@ -267,7 +256,12 @@ static int netcp_module_probe(struct netcp_device *netcp_device,
 	}
 
 	for_each_available_child_of_node(devices, child) {
-		const char *name = netcp_node_name(child);
+		const char *name;
+		char node_name[32];
+
+		if (of_property_read_string(node, "label", &name) < 0)
+			snprintf(node_name, sizeof(node_name), "%pOFn", child);
+			name = node_name;
 
 		if (!strcasecmp(module->name, name))
 			break;
@@ -2209,8 +2203,8 @@ static int netcp_probe(struct platform_device *pdev)
 	for_each_available_child_of_node(interfaces, child) {
 		ret = netcp_create_interface(netcp_device, child);
 		if (ret) {
-			dev_err(dev, "could not create interface(%s)\n",
-				child->name);
+			dev_err(dev, "could not create interface(%pOFn)\n",
+				child);
 			goto probe_quit_interface;
 		}
 	}
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 72b98e27c992..0397ccb6597e 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -3137,15 +3137,15 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
 	for_each_child_of_node(node, port) {
 		slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
 		if (!slave) {
-			dev_err(dev, "memory alloc failed for secondary port(%s), skipping...\n",
-				port->name);
+			dev_err(dev, "memory alloc failed for secondary port(%pOFn), skipping...\n",
+				port);
 			continue;
 		}
 
 		if (init_slave(gbe_dev, slave, port)) {
 			dev_err(dev,
-				"Failed to initialize secondary port(%s), skipping...\n",
-				port->name);
+				"Failed to initialize secondary port(%pOFn), skipping...\n",
+				port);
 			devm_kfree(dev, slave);
 			continue;
 		}
@@ -3239,8 +3239,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, XGBE_SS_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't xlate xgbe of node(%s) ss address at %d\n",
-			node->name, XGBE_SS_REG_INDEX);
+			"Can't xlate xgbe of node(%pOFn) ss address at %d\n",
+			node, XGBE_SS_REG_INDEX);
 		return ret;
 	}
 
@@ -3254,8 +3254,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, XGBE_SM_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't xlate xgbe of node(%s) sm address at %d\n",
-			node->name, XGBE_SM_REG_INDEX);
+			"Can't xlate xgbe of node(%pOFn) sm address at %d\n",
+			node, XGBE_SM_REG_INDEX);
 		return ret;
 	}
 
@@ -3269,8 +3269,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, XGBE_SERDES_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't xlate xgbe serdes of node(%s) address at %d\n",
-			node->name, XGBE_SERDES_REG_INDEX);
+			"Can't xlate xgbe serdes of node(%pOFn) address at %d\n",
+			node, XGBE_SERDES_REG_INDEX);
 		return ret;
 	}
 
@@ -3347,8 +3347,8 @@ static int get_gbe_resource_version(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, GBE_SS_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't translate of node(%s) of gbe ss address at %d\n",
-			node->name, GBE_SS_REG_INDEX);
+			"Can't translate of node(%pOFn) of gbe ss address at %d\n",
+			node, GBE_SS_REG_INDEX);
 		return ret;
 	}
 
@@ -3372,8 +3372,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, GBE_SGMII34_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't translate of gbe node(%s) address at index %d\n",
-			node->name, GBE_SGMII34_REG_INDEX);
+			"Can't translate of gbe node(%pOFn) address at index %d\n",
+			node, GBE_SGMII34_REG_INDEX);
 		return ret;
 	}
 
@@ -3388,8 +3388,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, GBE_SM_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't translate of gbe node(%s) address at index %d\n",
-			node->name, GBE_SM_REG_INDEX);
+			"Can't translate of gbe node(%pOFn) address at index %d\n",
+			node, GBE_SM_REG_INDEX);
 		return ret;
 	}
 
@@ -3498,8 +3498,8 @@ static int set_gbenu_ethss_priv(struct gbe_priv *gbe_dev,
 	ret = of_address_to_resource(node, GBENU_SM_REG_INDEX, &res);
 	if (ret) {
 		dev_err(gbe_dev->dev,
-			"Can't translate of gbenu node(%s) addr at index %d\n",
-			node->name, GBENU_SM_REG_INDEX);
+			"Can't translate of gbenu node(%pOFn) addr at index %d\n",
+			node, GBENU_SM_REG_INDEX);
 		return ret;
 	}
 
@@ -3642,7 +3642,7 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
 		ret = netcp_xgbe_serdes_init(gbe_dev->xgbe_serdes_regs,
 					     gbe_dev->ss_regs);
 	} else {
-		dev_err(dev, "unknown GBE node(%s)\n", node->name);
+		dev_err(dev, "unknown GBE node(%pOFn)\n", node);
 		ret = -ENODEV;
 	}
 
@@ -3667,8 +3667,8 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
 	for_each_child_of_node(interfaces, interface) {
 		ret = of_property_read_u32(interface, "slave-port", &slave_num);
 		if (ret) {
-			dev_err(dev, "missing slave-port parameter, skipping interface configuration for %s\n",
-				interface->name);
+			dev_err(dev, "missing slave-port parameter, skipping interface configuration for %pOFn\n",
+				interface);
 			continue;
 		}
 		gbe_dev->num_slaves++;
-- 
2.17.1

^ permalink raw reply related

* [Patch iproute2] ss: add UNIX_DIAG_VFS and UNIX_DIAG_ICONS for unix sockets
From: Cong Wang @ 2018-08-27 21:46 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Stephen Hemminger

UNIX_DIAG_VFS and UNIX_DIAG_ICONS are never used by ss,
make them available in ss -e output.

Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 misc/ss.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index 41e7762b..d28bc1ec 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -16,6 +16,7 @@
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
+#include <sys/sysmacros.h>
 #include <netinet/in.h>
 #include <string.h>
 #include <errno.h>
@@ -3604,6 +3605,28 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			out(" %c-%c",
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
+		if (tb[UNIX_DIAG_VFS]) {
+			struct unix_diag_vfs uv;
+
+			memcpy(&uv, RTA_DATA(tb[UNIX_DIAG_VFS]), sizeof(uv));
+			out(" ino:%u dev:%u/%u", uv.udiag_vfs_ino, major(uv.udiag_vfs_dev),
+						 minor(uv.udiag_vfs_dev));
+		}
+		if (tb[UNIX_DIAG_ICONS]) {
+			int len = RTA_PAYLOAD(tb[UNIX_DIAG_ICONS]);
+			__u32 *peers = malloc(len);
+			int i;
+
+			if (!peers) {
+				fprintf(stderr, "ss: failed to malloc buffer\n");
+				abort();
+			}
+			memcpy(peers, RTA_DATA(tb[UNIX_DIAG_ICONS]), len);
+			out(" peers:");
+			for (i = 0; i < len / sizeof(__u32); i++)
+				out(" %u", peers[i]);
+			free(peers);
+		}
 	}
 
 	return 0;
@@ -3641,6 +3664,8 @@ static int unix_show_netlink(struct filter *f)
 	req.r.udiag_show = UDIAG_SHOW_NAME | UDIAG_SHOW_PEER | UDIAG_SHOW_RQLEN;
 	if (show_mem)
 		req.r.udiag_show |= UDIAG_SHOW_MEMINFO;
+	if (show_details)
+		req.r.udiag_show |= UDIAG_SHOW_VFS | UDIAG_SHOW_ICONS;
 
 	return handle_netlink_request(f, &req.nlh, sizeof(req), unix_show_sock);
 }
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL
From: Cong Wang @ 2018-08-27 21:31 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Al Viro, Kees Cook, LKML, Jiri Pirko, David Miller,
	Linux Kernel Network Developers
In-Reply-To: <cb9d4e74-3498-48bd-45e0-05e925dbdb5b@mojatatu.com>

On Mon, Aug 27, 2018 at 4:58 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2018-08-26 6:57 p.m., Al Viro wrote:
> > On Sun, Aug 26, 2018 at 06:32:37PM +0100, Al Viro wrote:
> >
> >> As far as I can tell, the solution is
> > [snip long and painful reasoning]
> >> pointers, and not in provably opaque fashion.  Theoretically, the three tcf_...
> >> inlines above need another look; fortunately, they don't use ->next at all, not to
> >> mention not being used anywhere outside of net/sched/*.c
> >>
> >>      The 80 lines above prove that we only need to grep net/sched/*.c for
> >> tcf_proto_ops method calls.  And only because we don't have (thank $DEITY)
> >> anything that could deconstruct types - as soon as some bastard grows means
> >> to say "type of the second argument of the function pointed to by p", this
> >> kind of analysis, painful as it is, goes out of window.  Even as it is,
> >> do you really like the idea of newbies trying to get through the exercises
> >> like the one above?
> >
> > BTW, would there be any problem if we took the definitions of tcf_proto and
> > tcf_proto_ops to e.g. net/sched/tcf_proto.h (along with the three inlines in
> > in pkt_cls.h), left forwards in sch_generic.h and added includes of "tcf_proto.h"
> > where needed in net/sched/*.c?
> >
>
> I cant think of any challenges. Cong/Jiri? Would it require development
> time classifiers/actions/qdiscs to sit in that directory (I suspect you
> dont want them in include/net).
> BTW, the idea of improving grep-ability of the code by prefixing the
> ops appropriately makes sense. i.e we should have ops->cls_init,
> ops->act_init etc.

Hmm? Isn't struct tcf_proto_ops used and must be provided
by each tc filter module? How does it work if you move it into
net/sched/* for out-of-tree modules? Are they supposed to
include "..../net/sched/tcf_proto.h"?? Or something else?

BTW, we need some grep tool that really understands C syntax,
not making each variable friendly to plain grep.

^ permalink raw reply

* Did you get my message?
From: JACK EDISON @ 2018-08-27 21:30 UTC (permalink / raw)


Do you received my previous email? about your late relative having the
same surname with you which i previously sent to you? please Let me
know.

^ permalink raw reply

* Re: [V9fs-developer] [PATCH v2 2/2] 9p: Add refcount to p9_req_t
From: piaojun @ 2018-08-28  1:07 UTC (permalink / raw)
  To: Dominique Martinet, Tomas Bortoli
  Cc: lucho, Dominique Martinet, ericvh, netdev, linux-kernel,
	syzkaller, v9fs-developer, rminnich, davem
In-Reply-To: <20180827230954.GA21513@nautica>

Hi Dominique,

On 2018/8/28 7:09, Dominique Martinet wrote:
> Tomas Bortoli wrote on Tue, Aug 14, 2018:
>> To avoid use-after-free(s), use a refcount to keep track of the
>> usable references to any instantiated struct p9_req_t.
>>
>> This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
>> wrappers to kref_put(), kref_get() and kref_get_unless_zero().
>> These are used by the client and the transports to keep track of
>> valid requests' references.
>>
>> p9_free_req() is added back and used as callback by kref_put().
>>
>> Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
>> kmem_cache_free() will not be reused for another type until the rcu
>> synchronisation period is over, so an address gotten under rcu read
>> lock is safe to inc_ref() without corrupting random memory while
>> the lock is held.
> 
> 
> FWIW, since 4.19-rc1 has been tagged I was going to push this and all
> the perrequesites to linux-next, but I've managed to leak some requests
> by interrupting them in trans_virtio.
> I think I've found why (see below), so I'll push a fixed version after
> some more testing and another thorough read -- at some point today, but
> this hasn't been 'approved' explicitely so please review! :)
> 
> (Jun, I think you'll need to ask again to rename 'req' to 'rreq' if you
> think it's important -- I think such a rename should go in a separate
> patch anyway, there's plenty of time until the 4.20 merge window)
> 

I still think such a rename is necessary, and as you said, it will be
better go in another patch.

Thanks,
Jun

> 
> By "all the prerequesites" I mean this patch "serie":
> * 9p: Use a slab for allocating requests
> * 9p: Remove p9_idpool
> * net/9p: embed fcall in req to round down buffer allocs
> * net/9p: add a per-client fcall kmem_cache
> * 9p: rename p9_free_req() function
> * 9p: Add refcount to p9_req_t
> 
> All the other patchs have had some review though, I was just waiting for
> the start of this cycle, but if someone has any issue with the above
> patches now is a good time to say.
> 
> 
>> diff --git a/net/9p/client.c b/net/9p/client.c
>> index 7942c0bfcc5b..c9bb5d41afa4 100644
>> --- a/net/9p/client.c
>> +++ b/net/9p/client.c
>> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>>  
>>  	err = c->trans_mod->request(c, req);
>>  	if (err < 0) {
>> +		/* write won't happen */
>> +		p9_req_put(req);
>>  		if (err != -ERESTARTSYS && err != -EFAULT)
>>  			c->status = Disconnected;
>>  		goto recalc_sigpending;
> 
> p9_client_zc_rpc needs the same put if zc_request failed, I'm not sure
> why it wasn't here in my draft
> 

^ permalink raw reply

* [PATCH net] net/sched: act_pedit: fix dump of extended layered op
From: Davide Caratti @ 2018-08-27 20:56 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, David S. Miller; +Cc: netdev, Amir Vadai

in the (rare) case of failure in nla_nest_start(), missing NULL checks in
tcf_pedit_key_ex_dump() can make the following command

 # tc action add action pedit ex munge ip ttl set 64

dereference a NULL pointer:

 BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
 PGD 800000007d1cd067 P4D 800000007d1cd067 PUD 7acd3067 PMD 0
 Oops: 0002 [#1] SMP PTI
 CPU: 0 PID: 3336 Comm: tc Tainted: G            E     4.18.0.pedit+ #425
 Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
 RIP: 0010:tcf_pedit_dump+0x19d/0x358 [act_pedit]
 Code: be 02 00 00 00 48 89 df 66 89 44 24 20 e8 9b b1 fd e0 85 c0 75 46 8b 83 c8 00 00 00 49 83 c5 08 48 03 83 d0 00 00 00 4d 39 f5 <66> 89 04 25 00 00 00 00 0f 84 81 01 00 00 41 8b 45 00 48 8d 4c 24
 RSP: 0018:ffffb5d4004478a8 EFLAGS: 00010246
 RAX: ffff8880fcda2070 RBX: ffff8880fadd2900 RCX: 0000000000000000
 RDX: 0000000000000002 RSI: ffffb5d4004478ca RDI: ffff8880fcda206e
 RBP: ffff8880fb9cb900 R08: 0000000000000008 R09: ffff8880fcda206e
 R10: ffff8880fadd2900 R11: 0000000000000000 R12: ffff8880fd26cf40
 R13: ffff8880fc957430 R14: ffff8880fc957430 R15: ffff8880fb9cb988
 FS:  00007f75a537a740(0000) GS:ffff8880fda00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000000 CR3: 000000007a2fa005 CR4: 00000000001606f0
 Call Trace:
  ? __nla_reserve+0x38/0x50
  tcf_action_dump_1+0xd2/0x130
  tcf_action_dump+0x6a/0xf0
  tca_get_fill.constprop.31+0xa3/0x120
  tcf_action_add+0xd1/0x170
  tc_ctl_action+0x137/0x150
  rtnetlink_rcv_msg+0x263/0x2d0
  ? _cond_resched+0x15/0x40
  ? rtnl_calcit.isra.30+0x110/0x110
  netlink_rcv_skb+0x4d/0x130
  netlink_unicast+0x1a3/0x250
  netlink_sendmsg+0x2ae/0x3a0
  sock_sendmsg+0x36/0x40
  ___sys_sendmsg+0x26f/0x2d0
  ? do_wp_page+0x8e/0x5f0
  ? handle_pte_fault+0x6c3/0xf50
  ? __handle_mm_fault+0x38e/0x520
  ? __sys_sendmsg+0x5e/0xa0
  __sys_sendmsg+0x5e/0xa0
  do_syscall_64+0x5b/0x180
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7f75a4583ba0
 Code: c3 48 8b 05 f2 62 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d fd c3 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ae cc 00 00 48 89 04 24
 RSP: 002b:00007fff60ee7418 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
 RAX: ffffffffffffffda RBX: 00007fff60ee7540 RCX: 00007f75a4583ba0
 RDX: 0000000000000000 RSI: 00007fff60ee7490 RDI: 0000000000000003
 RBP: 000000005b842d3e R08: 0000000000000002 R09: 0000000000000000
 R10: 00007fff60ee6ea0 R11: 0000000000000246 R12: 0000000000000000
 R13: 00007fff60ee7554 R14: 0000000000000001 R15: 000000000066c100
 Modules linked in: act_pedit(E) ip6table_filter ip6_tables iptable_filter binfmt_misc crct10dif_pclmul ext4 crc32_pclmul mbcache ghash_clmulni_intel jbd2 pcbc snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd snd_timer cryptd glue_helper snd joydev pcspkr soundcore virtio_balloon i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c ata_generic pata_acpi virtio_net net_failover virtio_blk virtio_console failover qxl crc32c_intel drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix virtio_pci libata virtio_ring i2c_core virtio floppy dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_pedit]
 CR2: 0000000000000000

Like it's done for other TC actions, give up dumping pedit rules and return
an error if nla_nest_start() returns NULL.

Fixes: 71d0ed7079df ("net/act_pedit: Support using offset relative to the conventional network headers")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/sched/act_pedit.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 107034070019..ad99a99f11f6 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -109,16 +109,18 @@ static int tcf_pedit_key_ex_dump(struct sk_buff *skb,
 {
 	struct nlattr *keys_start = nla_nest_start(skb, TCA_PEDIT_KEYS_EX);
 
+	if (!keys_start)
+		goto nla_failure;
 	for (; n > 0; n--) {
 		struct nlattr *key_start;
 
 		key_start = nla_nest_start(skb, TCA_PEDIT_KEY_EX);
+		if (!key_start)
+			goto nla_failure;
 
 		if (nla_put_u16(skb, TCA_PEDIT_KEY_EX_HTYPE, keys_ex->htype) ||
-		    nla_put_u16(skb, TCA_PEDIT_KEY_EX_CMD, keys_ex->cmd)) {
-			nlmsg_trim(skb, keys_start);
-			return -EINVAL;
-		}
+		    nla_put_u16(skb, TCA_PEDIT_KEY_EX_CMD, keys_ex->cmd))
+			goto nla_failure;
 
 		nla_nest_end(skb, key_start);
 
@@ -128,6 +130,9 @@ static int tcf_pedit_key_ex_dump(struct sk_buff *skb,
 	nla_nest_end(skb, keys_start);
 
 	return 0;
+nla_failure:
+	nla_nest_cancel(skb, keys_start);
+	return -EINVAL;
 }
 
 static int tcf_pedit_init(struct net *net, struct nlattr *nla,
@@ -418,7 +423,10 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
 	opt->bindcnt = atomic_read(&p->tcf_bindcnt) - bind;
 
 	if (p->tcfp_keys_ex) {
-		tcf_pedit_key_ex_dump(skb, p->tcfp_keys_ex, p->tcfp_nkeys);
+		if (tcf_pedit_key_ex_dump(skb,
+					  p->tcfp_keys_ex,
+					  p->tcfp_nkeys))
+			goto nla_put_failure;
 
 		if (nla_put(skb, TCA_PEDIT_PARMS_EX, s, opt))
 			goto nla_put_failure;
-- 
2.17.1

^ permalink raw reply related

* Re: [patch net 0/2] net: sched: couple of small fixes
From: Cong Wang @ 2018-08-27 20:44 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Roman Mashak, mlxsw
In-Reply-To: <20180827185844.4517-1-jiri@resnulli.us>

On Mon, Aug 27, 2018 at 11:58 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@mellanox.com>
>
> Jiri Pirko (2):
>   net: sched: fix extack error message when chain is failed to be
>     created
>   net: sched: return -ENOENT when trying to remove filter from
>     non-existent chain

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

^ permalink raw reply

* Re: [PATCH v2 2/2] 9p: Add refcount to p9_req_t
From: Dominique Martinet @ 2018-08-27 23:09 UTC (permalink / raw)
  To: Tomas Bortoli
  Cc: ericvh, rminnich, lucho, davem, v9fs-developer, netdev,
	linux-kernel, syzkaller, Dominique Martinet
In-Reply-To: <20180814174342.11068-2-tomasbortoli@gmail.com>

Tomas Bortoli wrote on Tue, Aug 14, 2018:
> To avoid use-after-free(s), use a refcount to keep track of the
> usable references to any instantiated struct p9_req_t.
> 
> This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
> wrappers to kref_put(), kref_get() and kref_get_unless_zero().
> These are used by the client and the transports to keep track of
> valid requests' references.
> 
> p9_free_req() is added back and used as callback by kref_put().
> 
> Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
> kmem_cache_free() will not be reused for another type until the rcu
> synchronisation period is over, so an address gotten under rcu read
> lock is safe to inc_ref() without corrupting random memory while
> the lock is held.


FWIW, since 4.19-rc1 has been tagged I was going to push this and all
the perrequesites to linux-next, but I've managed to leak some requests
by interrupting them in trans_virtio.
I think I've found why (see below), so I'll push a fixed version after
some more testing and another thorough read -- at some point today, but
this hasn't been 'approved' explicitely so please review! :)

(Jun, I think you'll need to ask again to rename 'req' to 'rreq' if you
think it's important -- I think such a rename should go in a separate
patch anyway, there's plenty of time until the 4.20 merge window)


By "all the prerequesites" I mean this patch "serie":
* 9p: Use a slab for allocating requests
* 9p: Remove p9_idpool
* net/9p: embed fcall in req to round down buffer allocs
* net/9p: add a per-client fcall kmem_cache
* 9p: rename p9_free_req() function
* 9p: Add refcount to p9_req_t

All the other patchs have had some review though, I was just waiting for
the start of this cycle, but if someone has any issue with the above
patches now is a good time to say.


> diff --git a/net/9p/client.c b/net/9p/client.c
> index 7942c0bfcc5b..c9bb5d41afa4 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>  
>  	err = c->trans_mod->request(c, req);
>  	if (err < 0) {
> +		/* write won't happen */
> +		p9_req_put(req);
>  		if (err != -ERESTARTSYS && err != -EFAULT)
>  			c->status = Disconnected;
>  		goto recalc_sigpending;

p9_client_zc_rpc needs the same put if zc_request failed, I'm not sure
why it wasn't here in my draft

-- 
Dominique

^ permalink raw reply

* Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
From: Stephen Boyd @ 2018-08-27 19:14 UTC (permalink / raw)
  To: David S . Miller, Andy Shevchenko, Hans de Goede, Heiner Kallweit,
	Irina Tirdea, Michael Turquette
  Cc: netdev, Johannes Stezenbach, Carlo Caione, linux-clk
In-Reply-To: <0f52c05a-4f70-3022-3720-07c6bcf29ed8@redhat.com>

Quoting Hans de Goede (2018-08-27 11:53:19)
> On 27-08-18 20:47, Stephen Boyd wrote:
> > How would you know that a clk device driver hasn't probed yet and isn't
> > the driver that's actually providing the clk to this device on x86
> > systems? With DT systems we can figure that out by looking at the DT and
> > seeing if the device driver requesting the clk has the clocks property.
> > On x86 systems it's all clkdev which doesn't really lend itself to
> > solving this problem.
> 
> Right on x86 the assumption is that the clk driver will be builtin and
> will probe before the consumer. In this case that is true as the
> pmc-atom-clk driver can only be builtin and its platform device is
> instantiated from the acpi_lpss code and acpi init happens before
> the PCI bus is scanned.

If we can go with this assumption then we can make the optional clk API
work even on clkdev based systems. Maybe if x86 had some way of
indicating that all builtin clks are registered? That might work but
it's not very clean. Or if we could check to see if we're running on an
ACPI based system in clkdev we could use that to assume that clk_get()
will only be called after all providers have registered their lookups.

> 
> We have the same problem with ACPI OpRegions and drivers who have
> _PS0 / _PS3 methods using those OpRegions and the "solution" so far
> is the same, make sure all drivers providing OpRegion handlers are
> builtin.
> 
> Basically we (x86) miss the nice dependency info and complete hw
> description devicetree gives, so we rely on initialization order
> for some of this. I added the -EPROBE_DEFER handling for completeness
> sake / for other platforms, as you point out it will never trigger
> on x86.
> 

Thanks for the info!

^ permalink raw reply

* [PATCH 2/2] 9p: clear dangling pointers in p9stat_free
From: Dominique Martinet @ 2018-08-27 22:48 UTC (permalink / raw)
  To: v9fs-developer
  Cc: Dominique Martinet, linux-kernel, netdev, syzkaller-bugs,
	Eric Van Hensbergen, Latchesar Ionkov
In-Reply-To: <1535410108-20650-1-git-send-email-asmadeus@codewreck.org>

From: Dominique Martinet <dominique.martinet@cea.fr>

p9stat_free is more of a cleanup function than a 'free' function as it
only frees the content of the struct; there are chances of use-after-free
if it is improperly used (e.g. p9stat_free called twice as it used to be
possible to)

Clearing dangling pointers makes the function idempotent and safer to use.

Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
Reported-by: syzbot+d4252148d198410b864f@syzkaller.appspotmail.com
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Latchesar Ionkov <lucho@ionkov.net>
---
 net/9p/protocol.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index 4a1e1dd30b52..ee32bbf12675 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -46,10 +46,15 @@ p9pdu_writef(struct p9_fcall *pdu, int proto_version, const char *fmt, ...);
 void p9stat_free(struct p9_wstat *stbuf)
 {
 	kfree(stbuf->name);
+	stbuf->name = NULL;
 	kfree(stbuf->uid);
+	stbuf->uid = NULL;
 	kfree(stbuf->gid);
+	stbuf->gid = NULL;
 	kfree(stbuf->muid);
+	stbuf->muid = NULL;
 	kfree(stbuf->extension);
+	stbuf->extension = NULL;
 }
 EXPORT_SYMBOL(p9stat_free);
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH 1/2] v9fs_dir_readdir: fix double-free on p9stat_read error
From: Dominique Martinet @ 2018-08-27 22:48 UTC (permalink / raw)
  To: v9fs-developer
  Cc: Dominique Martinet, linux-kernel, netdev, syzkaller-bugs,
	Eric Van Hensbergen, Latchesar Ionkov
In-Reply-To: <000000000000af648b057456e234@google.com>

From: Dominique Martinet <dominique.martinet@cea.fr>

p9stat_read will call p9stat_free on error, we should only free the
struct content on success.

There also is no need to "p9stat_init" st as the read function will
zero the whole struct for us anyway, so clean up the code a bit while
we are here.

Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
Reported-by: syzbot+d4252148d198410b864f@syzkaller.appspotmail.com
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Latchesar Ionkov <lucho@ionkov.net>
---
 fs/9p/vfs_dir.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index b0405d6aac85..48db9a9f13f9 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -76,15 +76,6 @@ static inline int dt_type(struct p9_wstat *mistat)
 	return rettype;
 }
 
-static void p9stat_init(struct p9_wstat *stbuf)
-{
-	stbuf->name  = NULL;
-	stbuf->uid   = NULL;
-	stbuf->gid   = NULL;
-	stbuf->muid  = NULL;
-	stbuf->extension = NULL;
-}
-
 /**
  * v9fs_alloc_rdir_buf - Allocate buffer used for read and readdir
  * @filp: opened file structure
@@ -145,12 +136,10 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
 			rdir->tail = n;
 		}
 		while (rdir->head < rdir->tail) {
-			p9stat_init(&st);
 			err = p9stat_read(fid->clnt, rdir->buf + rdir->head,
 					  rdir->tail - rdir->head, &st);
 			if (err) {
 				p9_debug(P9_DEBUG_VFS, "returned %d\n", err);
-				p9stat_free(&st);
 				return -EIO;
 			}
 			reclen = st.size+2;
-- 
2.17.1

^ permalink raw reply related

* [patch net 2/2] net: sched: return -ENOENT when trying to remove filter from non-existent chain
From: Jiri Pirko @ 2018-08-27 18:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, mrv, mlxsw
In-Reply-To: <20180827185844.4517-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

When chain 0 was implicitly created, removal of non-existent filter from
chain 0 gave -ENOENT. Once chain 0 became non-implicit, the same call is
giving -EINVAL. Fix this by returning -ENOENT in that case.

Reported-by: Roman Mashak <mrv@mojatatu.com>
Fixes: f71e0ca4db18 ("net: sched: Avoid implicit chain 0 creation")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2d41c5b21b48..1a67af8a6e8c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1399,7 +1399,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 			goto errout;
 		}
 		NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
-		err = -EINVAL;
+		err = -ENOENT;
 		goto errout;
 	}
 
-- 
2.14.4

^ permalink raw reply related

* [patch net 1/2] net: sched: fix extack error message when chain is failed to be created
From: Jiri Pirko @ 2018-08-27 18:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, mrv, mlxsw
In-Reply-To: <20180827185844.4517-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Instead "Cannot find" say "Cannot create".

Fixes: c35a4acc2985 ("net: sched: cls_api: handle generic cls errors")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 31bd1439cf60..2d41c5b21b48 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1252,7 +1252,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	}
 	chain = tcf_chain_get(block, chain_index, true);
 	if (!chain) {
-		NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
+		NL_SET_ERR_MSG(extack, "Cannot create specified filter chain");
 		err = -ENOMEM;
 		goto errout;
 	}
-- 
2.14.4

^ permalink raw reply related

* [patch net 0/2] net: sched: couple of small fixes
From: Jiri Pirko @ 2018-08-27 18:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, mrv, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Jiri Pirko (2):
  net: sched: fix extack error message when chain is failed to be
    created
  net: sched: return -ENOENT when trying to remove filter from
    non-existent chain

 net/sched/cls_api.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.14.4

^ permalink raw reply

* Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
From: Hans de Goede @ 2018-08-27 18:53 UTC (permalink / raw)
  To: Stephen Boyd, David S . Miller, Andy Shevchenko, Heiner Kallweit,
	Irina Tirdea, Michael Turquette
  Cc: netdev, Johannes Stezenbach, Carlo Caione, linux-clk
In-Reply-To: <153539565488.129321.2586881199420174235@swboyd.mtv.corp.google.com>

Hi,

On 27-08-18 20:47, Stephen Boyd wrote:
> Quoting Hans de Goede (2018-08-27 07:31:58)
>> On some boards a platform clock is used as clock for the r8169 chip,
>> this commit adds support for getting and enabling this clock (assuming
>> it has an "ether_clk" alias set on it).
>>
>> This is related to commit d31fd43c0f9a ("clk: x86: Do not gate clocks
>> enabled by the firmware") which is a previous attempt to fix this for some
>> x86 boards, but this causes all Cherry Trail SoC using boards to not reach
>> there lowest power states when suspending.
>>
>> This commit (together with an atom-pmc-clk driver commit adding the alias)
>> fixes things properly by making the r8169 get the clock and enable it when
>> it needs it.
>>
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
>> Cc: Johannes Stezenbach <js@sig21.net>
>> Cc: Carlo Caione <carlo@endlessm.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Acked-by: Stephen Boyd <sboyd@kernel.org>

Thanks.

>> @@ -7614,6 +7616,11 @@ static void rtl_hw_initialize(struct rtl8169_private *tp)
>>          }
>>   }
>>   
>> +static void rtl_disable_clk(void *data)
>> +{
>> +       clk_disable_unprepare(data);
>> +}
>> +
>>   static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   {
>>          const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
>> @@ -7647,6 +7654,32 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>          mii->reg_num_mask = 0x1f;
>>          mii->supports_gmii = cfg->has_gmii;
>>   
>> +       /* Get the *optional* external "ether_clk" used on some boards */
>> +       tp->clk = devm_clk_get(&pdev->dev, "ether_clk");
> 
> An "optional" clk API is in flight, but it's easier to wait for that to
> merge and then this to be updated, so just take that as an FYI.

That is good to know.

> It would
> be interesting to see how to support optional clks with clkdev lookup > though, which would be needed in this case.

Ack.

> How would you know that a clk device driver hasn't probed yet and isn't
> the driver that's actually providing the clk to this device on x86
> systems? With DT systems we can figure that out by looking at the DT and
> seeing if the device driver requesting the clk has the clocks property.
> On x86 systems it's all clkdev which doesn't really lend itself to
> solving this problem.

Right on x86 the assumption is that the clk driver will be builtin and
will probe before the consumer. In this case that is true as the
pmc-atom-clk driver can only be builtin and its platform device is
instantiated from the acpi_lpss code and acpi init happens before
the PCI bus is scanned.

We have the same problem with ACPI OpRegions and drivers who have
_PS0 / _PS3 methods using those OpRegions and the "solution" so far
is the same, make sure all drivers providing OpRegion handlers are
builtin.

Basically we (x86) miss the nice dependency info and complete hw
description devicetree gives, so we rely on initialization order
for some of this. I added the -EPROBE_DEFER handling for completeness
sake / for other platforms, as you point out it will never trigger
on x86.

Regards,

Hans




> 
>> +       if (IS_ERR(tp->clk)) {
>> +               rc = PTR_ERR(tp->clk);

^ permalink raw reply

* Re: KASAN: invalid-free in p9stat_free
From: Dominique Martinet @ 2018-08-27 22:40 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: syzbot, David Miller, Eric Van Hensbergen, LKML, Latchesar Ionkov,
	netdev, syzkaller-bugs, v9fs-developer
In-Reply-To: <CACT4Y+b1B5_Eg+j91GxaVbPk5sb8+SVOU_z5k9HYb4R716X9kQ@mail.gmail.com>

Dmitry Vyukov wrote on Mon, Aug 27, 2018:
> kfree and then null pointer is pretty common, try to run:
> 
> find -name "*.c" -exec grep -A 1 "kfree(" {} \; | grep -B 1 " = NULL;"

Hmm, right, it looks like somewhere between 5 and 10% of the kfree()
calls are followed by NULL assignment, that's "common enough" - not
generalized but not rare either.

> Leaving dangling pointers behind is not the best idea.
> And from what I remember a bunch of similar double frees were fixed by
> nulling the pointer after the first kfree.

In this case it really is an error to call p9stat_free again, so let's
just do both.
Will send the patches shortly.


Thanks,
-- 
Dominique

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox