Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next RFC 7/8] udp: gro behind static key
From: Subash Abhinov Kasiviswanathan @ 2018-09-15  3:37 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, pabeni, steffen.klassert, davem, Willem de Bruijn
In-Reply-To: <20180914175941.213950-8-willemdebruijn.kernel@gmail.com>

On 2018-09-14 11:59, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> gro callback configured.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4f6aa95a9b12..f44fe328aa0f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct
> list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
> 
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) ||
> !static_branch_unlikely(&udp_encap_needed_key))
>  		goto flush;
> 

Hi Willem

Does this need to be

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 6dd3f0a..fcd5589 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -407,7 +407,7 @@ static struct sk_buff *udp4_gro_receive(struct 
list_head *head,
  {
         struct udphdr *uh = udp_gro_udphdr(skb);

-       if (unlikely(!uh) || 
!static_branch_unlikely(&udp_encap_needed_key))
+       if (unlikely(!uh) || 
static_branch_unlikely(&udp_encap_needed_key))
                 goto flush;

         /* Don't bother verifying checksum if we're going to flush 
anyway. */

I tried setting UDP_GRO socket option and I had to make this change to
exercise the udp_gro_receive_cb code path.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: Need input on placement of driver
From: Sunil Kovvuri @ 2018-09-15  3:40 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List, Arnd Bergmann
In-Reply-To: <CA+sq2Cdj=tVxnk8RgwjKTTYMQbQpOjNLkKU8BPRph_i4Ri+0=Q@mail.gmail.com>

On Wed, Sep 12, 2018 at 10:20 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> Hi David,
>
> I am trying to submit a driver into drivers/soc folder and Arnd is of
> the opinion that
> the driver should be moved to drivers/net/ethernet.
>
> Can you please go through below and give your feedback.
>
> HW functionality in brief
> # HW has a Admin function (AF) PCI device which has privilege access
> to configure co-processors.
> # Co-processors include network block, crypto block, ring buffer block
> used by both network and crypto blocks, packet or anyother work
> scheduler block, ingress/egress packet parser and forwarding block,
> internal state machine caches etc.
> # Each of these blocks are multiple in number and can be attached to
> other PCI devices.
> # Future variants of the same silicon might have additional functional
> blocks in AF.
> # There are other SRIOV PF/VF devices which are dumb at power-on and
> acquire functionality based
>     what blocks are attached to them by AF.
> # So AF is the one which configures, facilities and manages all HW
> resources (network and non-network).
>    But doesn't handle any data.
> # PF/VFs communicate with AF via a shared mailbox memory for functional block
>    attach / detach requests, HW configuration etc etc.
>
> AF driver will have logic not only the functionality needed by kernel
> netdev or crypto drivers but
> also the HW configuration logic needed by userspace application drivers.
>
> Keeping current and future functionality in view we thought of having
> 3 drivers in kernel
> # AF driver at drivers/soc
> # PF/ VF netdev driver (network & ring buffer blocks attached to these
> devices) at drivers/net/ethernet
> # PF / VF crypto driver (ring buffer and crypto blocks attached to
> these devices) at drivers/crypto.
>
> I have submitted few patches for the AF driver
> https://patchwork.kernel.org/cover/10587635/
>
> Here Arnd has opined that all drivers should move into drivers/net/ethernet.
> So wanted to check if you would be okay with this.
>
> Thanks,
> Sunil.


Hi David,

Sorry for the reminder.
It would be great to have your feedback / inputs.

Thanks,
Sunil.

^ permalink raw reply

* [PATCH] net: socionext: Fix two sleep-in-atomic-context bugs in ave_rxfifo_reset()
From: Jia-Ju Bai @ 2018-09-15  4:02 UTC (permalink / raw)
  To: davem, hayashi.kunihiko, yamada.masahiro, robh, andrew
  Cc: netdev, linux-kernel, Jia-Ju Bai

The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.17 are:

[FUNC] usleep_range
drivers/net/ethernet/socionext/sni_ave.c, 892: 
	usleep_range in ave_rxfifo_reset
drivers/net/ethernet/socionext/sni_ave.c, 932: 
	ave_rxfifo_reset in ave_irq_handler

[FUNC] usleep_range
drivers/net/ethernet/socionext/sni_ave.c, 888: 
	usleep_range in ave_rxfifo_reset
drivers/net/ethernet/socionext/sni_ave.c, 932: 
	ave_rxfifo_reset in ave_irq_handler

To fix these bugs, usleep_range() is replaced with udelay().

These bugs are found by my static analysis tool DSAC.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 drivers/net/ethernet/socionext/sni_ave.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index f7ecceeb1e28..3c784a29ce0d 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -904,11 +904,11 @@ static void ave_rxfifo_reset(struct net_device *ndev)
 
 	/* assert reset */
 	writel(AVE_GRR_RXFFR, priv->base + AVE_GRR);
-	usleep_range(40, 50);
+	udelay(50);
 
 	/* negate reset */
 	writel(0, priv->base + AVE_GRR);
-	usleep_range(10, 20);
+	udelay(20);
 
 	/* negate interrupt status */
 	writel(AVE_GI_RXOVF, priv->base + AVE_GISR);
-- 
2.17.0

^ permalink raw reply related

* [PATCH net-next] net/smc: cast sizeof to int for comparison
From: YueHaibing @ 2018-09-15 10:00 UTC (permalink / raw)
  To: davem, ubraun; +Cc: linux-kernel, netdev, linux-s390, YueHaibing

Comparing an int to a size, which is unsigned, causes the int to become
unsigned, giving the wrong result. kernel_sendmsg can return a negative
error code.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/smc/smc_clc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 83aba9a..fd0f5ce 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -446,7 +446,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, int smc_type,
 	vec[i++].iov_len = sizeof(trl);
 	/* due to the few bytes needed for clc-handshake this cannot block */
 	len = kernel_sendmsg(smc->clcsock, &msg, vec, i, plen);
-	if (len < sizeof(pclc)) {
+	if (len < (int)sizeof(pclc)) {
 		if (len >= 0) {
 			reason_code = -ENETUNREACH;
 			smc->sk.sk_err = -reason_code;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next] net/smc: cast sizeof to int for comparison
From: Andreas Schwab @ 2018-09-15 11:35 UTC (permalink / raw)
  To: YueHaibing; +Cc: davem, ubraun, linux-kernel, netdev, linux-s390
In-Reply-To: <20180915100036.20100-1-yuehaibing@huawei.com>

On Sep 15 2018, YueHaibing <yuehaibing@huawei.com> wrote:

> Comparing an int to a size, which is unsigned, causes the int to become
> unsigned, giving the wrong result. kernel_sendmsg can return a negative
> error code.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  net/smc/smc_clc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 83aba9a..fd0f5ce 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -446,7 +446,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, int smc_type,
>  	vec[i++].iov_len = sizeof(trl);
>  	/* due to the few bytes needed for clc-handshake this cannot block */
>  	len = kernel_sendmsg(smc->clcsock, &msg, vec, i, plen);
> -	if (len < sizeof(pclc)) {
> +	if (len < (int)sizeof(pclc)) {
>  		if (len >= 0) {
>  			reason_code = -ENETUNREACH;
>  			smc->sk.sk_err = -reason_code;

It would perhaps be better to handle len < 0 first.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: gswip: Fix return value check in gswip_probe()
From: Hauke Mehrtens @ 2018-09-15  9:14 UTC (permalink / raw)
  To: Wei Yongjun, Andrew Lunn, Vivien Didelot, Florian Fainelli
  Cc: netdev, kernel-janitors
In-Reply-To: <1536975201-184967-1-git-send-email-weiyongjun1@huawei.com>


[-- Attachment #1.1: Type: text/plain, Size: 1585 bytes --]

On 09/15/2018 03:33 AM, Wei Yongjun wrote:
> In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> and never returns NULL. The NULL test in the return value check should
> be replaced with IS_ERR().
> 
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Acked-by: Hauke Mehrtens <hauke@hauke-m.de>

> ---
>  drivers/net/dsa/lantiq_gswip.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 9c28d0b..faac359 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -1044,18 +1044,18 @@ static int gswip_probe(struct platform_device *pdev)
>  
>  	gswip_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	priv->gswip = devm_ioremap_resource(dev, gswip_res);
> -	if (!priv->gswip)
> -		return -ENOMEM;
> +	if (IS_ERR(priv->gswip))
> +		return PTR_ERR(priv->gswip);
>  
>  	mdio_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	priv->mdio = devm_ioremap_resource(dev, mdio_res);
> -	if (!priv->mdio)
> -		return -ENOMEM;
> +	if (IS_ERR(priv->mdio))
> +		return PTR_ERR(priv->mdio);
>  
>  	mii_res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>  	priv->mii = devm_ioremap_resource(dev, mii_res);
> -	if (!priv->mii)
> -		return -ENOMEM;
> +	if (IS_ERR(priv->mii))
> +		return PTR_ERR(priv->mii);
>  
>  	priv->hw_info = of_device_get_match_data(dev);
>  	if (!priv->hw_info)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: gswip: Fix copy-paste error in gswip_gphy_fw_probe()
From: Hauke Mehrtens @ 2018-09-15  9:14 UTC (permalink / raw)
  To: Wei Yongjun, Andrew Lunn, Vivien Didelot, Florian Fainelli
  Cc: netdev, kernel-janitors
In-Reply-To: <1536975218-185034-1-git-send-email-weiyongjun1@huawei.com>


[-- Attachment #1.1: Type: text/plain, Size: 1223 bytes --]

On 09/15/2018 03:33 AM, Wei Yongjun wrote:
> The return value from of_reset_control_array_get_exclusive() is not
> checked correctly. The test is done against a wrong variable. This
> patch fix it.
> 
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Acked-by: Hauke Mehrtens <hauke@hauke-m.de>

> ---
>  drivers/net/dsa/lantiq_gswip.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 9c28d0b..9c10570 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -934,10 +934,10 @@ static int gswip_gphy_fw_probe(struct gswip_priv *priv,
>  	}
>  
>  	gphy_fw->reset = of_reset_control_array_get_exclusive(gphy_fw_np);
> -	if (IS_ERR(priv->gphy_fw)) {
> -		if (PTR_ERR(priv->gphy_fw) != -EPROBE_DEFER)
> +	if (IS_ERR(gphy_fw->reset)) {
> +		if (PTR_ERR(gphy_fw->reset) != -EPROBE_DEFER)
>  			dev_err(dev, "Failed to lookup gphy reset\n");
> -		return PTR_ERR(priv->gphy_fw);
> +		return PTR_ERR(gphy_fw->reset);
>  	}
>  
>  	return gswip_gphy_fw_load(priv, gphy_fw);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] net: lantiq: Fix return value check in xrx200_probe()
From: Hauke Mehrtens @ 2018-09-15  9:15 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: netdev, kernel-janitors
In-Reply-To: <1536975230-185101-1-git-send-email-weiyongjun1@huawei.com>


[-- Attachment #1.1: Type: text/plain, Size: 1160 bytes --]

On 09/15/2018 03:33 AM, Wei Yongjun wrote:
> In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> and never returns NULL. The NULL test in the return value check should
> be replaced with IS_ERR().
> 
> Fixes: fe1a56420cf2 ("net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Acked-by: Hauke Mehrtens <hauke@hauke-m.de>

> ---
>  drivers/net/ethernet/lantiq_xrx200.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
> index c8b6d90..4a16076 100644
> --- a/drivers/net/ethernet/lantiq_xrx200.c
> +++ b/drivers/net/ethernet/lantiq_xrx200.c
> @@ -461,9 +461,9 @@ static int xrx200_probe(struct platform_device *pdev)
>  	}
>  
>  	priv->pmac_reg = devm_ioremap_resource(dev, res);
> -	if (!priv->pmac_reg) {
> +	if (IS_ERR(priv->pmac_reg)) {
>  		dev_err(dev, "failed to request and remap io ranges\n");
> -		return -ENOMEM;
> +		return PTR_ERR(priv->pmac_reg);
>  	}
>  
>  	priv->chan_rx.dma.irq = platform_get_irq_byname(pdev, "rx");
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATH RFC net-next 6/8] net: ethernet xgbe expand PHY_GBIT_FEAUTRES
From: Sergei Shtylyov @ 2018-09-15 10:01 UTC (permalink / raw)
  To: Andrew Lunn, netdev; +Cc: Florian Fainelli
In-Reply-To: <1536961136-30453-7-git-send-email-andrew@lunn.ch>

On 9/15/2018 12:38 AM, Andrew Lunn wrote:

> The macro PHY_GBIT_FEAUTRES needs to change into a bitmap in order to
> support link_modes. Remove its use from xgde by replacing it with its
> definition.
> 
> Probably, the current behavior is wrong. It probably should be
> ANDing not assigning.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> index a7e03e3ecc93..d49e76982453 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> @@ -878,8 +878,9 @@ static bool xgbe_phy_finisar_phy_quirks(struct xgbe_prv_data *pdata)
>   	phy_write(phy_data->phydev, 0x04, 0x0d01);
>   	phy_write(phy_data->phydev, 0x00, 0x9140);
>   
> -	phy_data->phydev->supported = PHY_GBIT_FEATURES;
> -	phy_data->phydev->advertising = phy_data->phydev->supported;
> +	phy_data->phydev->supported = (PHY_10BT_FEATURES |
> +				       PHY_100BT_FEATURES |
> +				       PHY_1000BT_FEATURES);

    Parens not necessary here.

>   	phy_support_asym_pause(phy_data->phydev);
>   
>   	netif_dbg(pdata, drv, pdata->netdev,
> @@ -950,8 +951,9 @@ static bool xgbe_phy_belfuse_phy_quirks(struct xgbe_prv_data *pdata)
>   	reg = phy_read(phy_data->phydev, 0x00);
>   	phy_write(phy_data->phydev, 0x00, reg & ~0x00800);
>   
> -	phy_data->phydev->supported = PHY_GBIT_FEATURES;
> -	phy_data->phydev->advertising = phy_data->phydev->supported;
> +	phy_data->phydev->supported = (PHY_10BT_FEATURES |
> +				       PHY_100BT_FEATURES |
> +				       PHY_1000BT_FEATURES);

    Here as well.

[...]

MBR, Sergei

^ permalink raw reply

* URG­ENT UPD­ATES.................
From: Fatou Bobo Wann @ 2018-09-15 11:05 UTC (permalink / raw)

In-Reply-To: <493893948.2187709.1537009507959.ref@mail.yahoo.com>

Dear Friend,

With due respect to your person and much sincerity of purpose.I wrote you two days ago expecting that you would reply as regards the urgency of this proposal that i passed to you.Anyway, I don't know if you did receive it or not that is the reason I am re-sending it.I have a business proposal which I will like to handle with you. $35 million USD is involves. But be rest assured that everything is legal and risk free as I have concluded all the arrangements and the legal papers that will back the transaction up. Kindly indicate your interest as to enable me tell you more detail of the proposal.

Waiting for your urgent response.
Yours Faithfully,
Mrs.Fatou Bobo Wann

^ permalink raw reply

* [PATCH net-next 0/5] net: lantiq: Minor fixes for vrx200 and gswip
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens

These are mostly minor fixes to problems addresses in the latests round 
of the review of the original series adding these driver, which were not 
applied before the patches got merged into net-next.
In addition it fixes a data bus error on poweroff.

Hauke Mehrtens (5):
  dt-bindings: net: lantiq,xrx200-net: Use lower case in hex
  dt-bindings: net: dsa: lantiq,xrx200-gswip: Fix minor style fixes
  net: lantiq: lantiq_xrx200: Move clock prepare to probe function
  net: dsa: lantiq_gswip: Minor code style improvements
  net: dsa: tag_gswip: Add gswip to dsa_tag_protocol_to_str()

 .../devicetree/bindings/net/dsa/lantiq-gswip.txt   | 18 +++++-----
 .../devicetree/bindings/net/lantiq,xrx200-net.txt  |  4 +--
 drivers/net/dsa/lantiq_gswip.c                     | 38 ++++++++++------------
 drivers/net/ethernet/lantiq_xrx200.c               | 21 +++++++-----
 net/dsa/dsa.c                                      |  3 ++
 5 files changed, 45 insertions(+), 39 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH net-next 1/5] dt-bindings: net: lantiq,xrx200-net: Use lower case in hex
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>

Use lower case letters in the addresses of the device tree binding.
In addition replace eth with ethernet and fix the size of the reg
element in the example. The additional range does not contain any
registers but is used for the IP block on the this SoC.

Fixes: 839790e88a3c ("dt-bindings: net: Add lantiq, xrx200-net DT bindings")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
index 8a2fe5200cdc..5ff5e68bbbb6 100644
--- a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
+++ b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
@@ -11,11 +11,11 @@ Required properties:
 
 Example:
 
-eth0: eth@E10B308 {
+ethernet@e10b308 {
 	#address-cells = <1>;
 	#size-cells = <0>;
 	compatible = "lantiq,xrx200-net";
-	reg = <0xE10B308 0x30>;
+	reg = <0xe10b308 0xcf8>;
 	interrupts = <73>, <72>;
 	interrupt-names = "tx", "rx";
 };
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 4/5] net: dsa: lantiq_gswip: Minor code style improvements
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>

Use one code block when returning because the interface type is
unsupported and also check if some unsupported port gets configured.
In addition fix a double the and use dsa_is_cpu_port() instated of
manually getting the CPU port.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/dsa/lantiq_gswip.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 9c28d0b2fcdb..4e3732b5790f 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -615,32 +615,24 @@ static void gswip_phylink_validate(struct dsa_switch *ds, int port,
 		if (!phy_interface_mode_is_rgmii(state->interface) &&
 		    state->interface != PHY_INTERFACE_MODE_MII &&
 		    state->interface != PHY_INTERFACE_MODE_REVMII &&
-		    state->interface != PHY_INTERFACE_MODE_RMII) {
-			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-			dev_err(ds->dev,
-			"Unsupported interface: %d\n", state->interface);
-			return;
-		}
+		    state->interface != PHY_INTERFACE_MODE_RMII)
+			goto unsupported;
 		break;
 	case 2:
 	case 3:
 	case 4:
-		if (state->interface != PHY_INTERFACE_MODE_INTERNAL) {
-			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-			dev_err(ds->dev,
-			"Unsupported interface: %d\n", state->interface);
-			return;
-		}
+		if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
+			goto unsupported;
 		break;
 	case 5:
 		if (!phy_interface_mode_is_rgmii(state->interface) &&
-		    state->interface != PHY_INTERFACE_MODE_INTERNAL) {
-			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-			dev_err(ds->dev,
-			"Unsupported interface: %d\n", state->interface);
-			return;
-		}
+		    state->interface != PHY_INTERFACE_MODE_INTERNAL)
+			goto unsupported;
 		break;
+	default:
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		dev_err(ds->dev, "Unsupported port: %i\n", port);
+		return;
 	}
 
 	/* Allow all the expected bits */
@@ -667,6 +659,12 @@ static void gswip_phylink_validate(struct dsa_switch *ds, int port,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	return;
+
+unsupported:
+	bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+	dev_err(ds->dev, "Unsupported interface: %d\n", state->interface);
+	return;
 }
 
 static void gswip_phylink_mac_config(struct dsa_switch *ds, int port,
@@ -970,7 +968,7 @@ static int gswip_gphy_fw_list(struct gswip_priv *priv,
 	int err;
 	int i = 0;
 
-	/* The The VRX200 rev 1.1 uses the GSWIP 2.0 and needs the older
+	/* The VRX200 rev 1.1 uses the GSWIP 2.0 and needs the older
 	 * GPHY firmware. The VRX200 rev 1.2 uses the GSWIP 2.1 and also
 	 * needs a different GPHY firmware.
 	 */
@@ -1097,7 +1095,7 @@ static int gswip_probe(struct platform_device *pdev)
 		dev_err(dev, "dsa switch register failed: %i\n", err);
 		goto mdio_bus;
 	}
-	if (priv->ds->dst->cpu_dp->index != priv->hw_info->cpu_port) {
+	if (!dsa_is_cpu_port(priv->ds, priv->hw_info->cpu_port)) {
 		dev_err(dev, "wrong CPU port defined, HW only supports port: %i",
 			priv->hw_info->cpu_port);
 		err = -EINVAL;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 2/5] dt-bindings: net: dsa: lantiq,xrx200-gswip: Fix minor style fixes
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>

* Use one compatible line per line in the documentation
* Remove SoC revision depended compatible lines, we can detect that in
  the driver
* Use lower case letters in hex addresses
* Fix the size of the address ranges in the example, this now matches
  the sizes used by the SoC. The old ones will also work, this just adds
  some empty address space.
* Change the reg size of the gphy-fw node

Fixes: 86ce2bc73c7a ("dt-bindings: net: dsa: Add lantiq, xrx200-gswip DT bindings")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/net/dsa/lantiq-gswip.txt       | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/lantiq-gswip.txt b/Documentation/devicetree/bindings/net/dsa/lantiq-gswip.txt
index a089f5856778..886cbe8ffb38 100644
--- a/Documentation/devicetree/bindings/net/dsa/lantiq-gswip.txt
+++ b/Documentation/devicetree/bindings/net/dsa/lantiq-gswip.txt
@@ -22,9 +22,9 @@ required and optional properties.
 
 
 Required properties for GPHY firmware loading:
-- compatible	: "lantiq,gphy-fw" and "lantiq,xrx200-gphy-fw",
-		  "lantiq,xrx200a1x-gphy-fw", "lantiq,xrx200a2x-gphy-fw",
-		  "lantiq,xrx300-gphy-fw", or "lantiq,xrx330-gphy-fw"
+- compatible	: "lantiq,xrx200-gphy-fw", "lantiq,gphy-fw"
+		  "lantiq,xrx300-gphy-fw", "lantiq,gphy-fw"
+		  "lantiq,xrx330-gphy-fw", "lantiq,gphy-fw"
 		  for the loading of the firmware into the embedded
 		  GPHY core of the SoC.
 - lantiq,rcu	: reference to the rcu syscon
@@ -41,13 +41,13 @@ Example:
 
 Ethernet switch on the VRX200 SoC:
 
-gswip: gswip@E108000 {
+switch@e108000 {
 	#address-cells = <1>;
 	#size-cells = <0>;
 	compatible = "lantiq,xrx200-gswip";
-	reg = <	0xE108000 0x3000 /* switch */
-		0xE10B100 0x70 /* mdio */
-		0xE10B1D8 0x30 /* mii */
+	reg = <	0xe108000 0x3100	/* switch */
+		0xe10b100 0xd8		/* mdio */
+		0xe10b1d8 0x130		/* mii */
 		>;
 	dsa,member = <0 0>;
 
@@ -97,7 +97,7 @@ gswip: gswip@E108000 {
 		};
 	};
 
-	mdio@0 {
+	mdio {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "lantiq,xrx200-mdio";
@@ -123,6 +123,8 @@ gswip: gswip@E108000 {
 	gphy-fw {
 		compatible = "lantiq,xrx200-gphy-fw", "lantiq,gphy-fw";
 		lantiq,rcu = <&rcu0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 
 		gphy@20 {
 			reg = <0x20>;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 3/5] net: lantiq: lantiq_xrx200: Move clock prepare to probe function
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>

The switch and the MAC are in one IP core and they use the same clock
signal from the clock generation unit.
Currently the clock architecture in the lantiq SoC code does not allow
to easily share the same clocks, this has to be fixed by switching to
the common clock framework.
As a workaround the clock of the switch and MAC should be activated when
the MAC gets probed and only disabled when the MAC gets removed. This
way it is ensured that the clock is always enabled when the switch or
MAC is used. The switch can not be used without the MAC.

This fixes a data bus error when rebooting the system and deactivating
the switch and mac and later accessing some registers in the cleanup
while the clocks are disabled.

Fixes: fe1a56420cf2 ("net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/lantiq_xrx200.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index c8b6d908f0cc..14b20ce0dd43 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -115,12 +115,6 @@ static void xrx200_flush_dma(struct xrx200_chan *ch)
 static int xrx200_open(struct net_device *net_dev)
 {
 	struct xrx200_priv *priv = netdev_priv(net_dev);
-	int err;
-
-	/* enable clock gate */
-	err = clk_prepare_enable(priv->clk);
-	if (err)
-		return err;
 
 	napi_enable(&priv->chan_tx.napi);
 	ltq_dma_open(&priv->chan_tx.dma);
@@ -155,8 +149,6 @@ static int xrx200_close(struct net_device *net_dev)
 	napi_disable(&priv->chan_tx.napi);
 	ltq_dma_close(&priv->chan_tx.dma);
 
-	clk_disable_unprepare(priv->clk);
-
 	return 0;
 }
 
@@ -497,6 +489,11 @@ static int xrx200_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* enable clock gate */
+	err = clk_prepare_enable(priv->clk);
+	if (err)
+		goto err_uninit_dma;
+
 	/* set IPG to 12 */
 	xrx200_pmac_mask(priv, PMAC_RX_IPG_MASK, 0xb, PMAC_RX_IPG);
 
@@ -514,9 +511,12 @@ static int xrx200_probe(struct platform_device *pdev)
 
 	err = register_netdev(net_dev);
 	if (err)
-		goto err_uninit_dma;
+		goto err_unprepare_clk;
 	return err;
 
+err_unprepare_clk:
+	clk_disable_unprepare(priv->clk);
+
 err_uninit_dma:
 	xrx200_hw_cleanup(priv);
 
@@ -536,6 +536,9 @@ static int xrx200_remove(struct platform_device *pdev)
 	/* remove the actual device */
 	unregister_netdev(net_dev);
 
+	/* release the clock */
+	clk_disable_unprepare(priv->clk);
+
 	/* shut down hardware */
 	xrx200_hw_cleanup(priv);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 5/5] net: dsa: tag_gswip: Add gswip to dsa_tag_protocol_to_str()
From: Hauke Mehrtens @ 2018-09-15 12:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree, Hauke Mehrtens
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>

The gswip tag was missing in the dsa_tag_protocol_to_str() function, add it.

Fixes: 7969119293f5 ("net: dsa: Add Lantiq / Intel GSWIP tag support")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 net/dsa/dsa.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 5f73e96cc9e6..a69c1790bbfc 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -88,6 +88,9 @@ const char *dsa_tag_protocol_to_str(const struct dsa_device_ops *ops)
 #ifdef CONFIG_NET_DSA_TAG_EDSA
 		[DSA_TAG_PROTO_EDSA] = "edsa",
 #endif
+#ifdef CONFIG_NET_DSA_TAG_GSWIP
+		[DSA_TAG_PROTO_GSWIP] = "gswip",
+#endif
 #ifdef CONFIG_NET_DSA_TAG_KSZ
 		[DSA_TAG_PROTO_KSZ] = "ksz",
 #endif
-- 
2.11.0

^ permalink raw reply related

* RE: [PATCH net-next v2] net/tls: Add support for async decryption of tls records
From: Vakul Garg @ 2018-09-15 12:11 UTC (permalink / raw)
  To: John Fastabend, netdev@vger.kernel.org
  Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
	davem@davemloft.net
In-Reply-To: <e36c42e9-17ed-6f9b-809a-5f1a0cb1d9de@gmail.com>



> -----Original Message-----
> From: John Fastabend <john.fastabend@gmail.com>
> Sent: Saturday, September 15, 2018 1:10 AM
> To: Vakul Garg <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: borisp@mellanox.com; aviadye@mellanox.com; davejwatson@fb.com;
> davem@davemloft.net
> Subject: Re: [PATCH net-next v2] net/tls: Add support for async decryption of
> tls records
> 
> On 08/29/2018 02:56 AM, Vakul Garg wrote:
> > When tls records are decrypted using asynchronous acclerators such as
> > NXP CAAM engine, the crypto apis return -EINPROGRESS. Presently, on
> > getting -EINPROGRESS, the tls record processing stops till the time
> > the crypto accelerator finishes off and returns the result. This
> > incurs a context switch and is not an efficient way of accessing the
> > crypto accelerators. Crypto accelerators work efficient when they are
> > queued with multiple crypto jobs without having to wait for the
> > previous ones to complete.
> >
> > The patch submits multiple crypto requests without having to wait for
> > for previous ones to complete. This has been implemented for records
> > which are decrypted in zero-copy mode. At the end of recvmsg(), we
> > wait for all the asynchronous decryption requests to complete.
> >
> > The references to records which have been sent for async decryption
> > are dropped. For cases where record decryption is not possible in
> > zero-copy mode, asynchronous decryption is not used and we wait for
> > decryption crypto api to complete.
> >
> > For crypto requests executing in async fashion, the memory for
> > aead_request, sglists and skb etc is freed from the decryption
> > completion handler. The decryption completion handler wakesup the
> > sleeping user context when recvmsg() flags that it has done sending
> > all the decryption requests and there are no more decryption requests
> > pending to be completed.
> >
> > Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> > Reviewed-by: Dave Watson <davejwatson@fb.com>
> > ---
> 
> [...]
> 
> 
> > @@ -1271,6 +1377,8 @@ int tls_set_sw_offload(struct sock *sk, struct
> tls_context *ctx, int tx)
> >  		goto free_aead;
> >
> >  	if (sw_ctx_rx) {
> > +		(*aead)->reqsize = sizeof(struct decrypt_req_ctx);
> > +
> 
> This is not valid and may cause GPF or best case only a KASAN warning.
> 'reqsize' should probably not be mangled outside the internal crypto APIs but
> the real reason is the reqsize is used to determine how much space is needed
> at the end of the aead_request for crypto private ctx use in encrypt/decrypt.
> After this patch when we submit an aead_request the crypto layer will think it
> has room for its private structs at the end but now only 8B will be there and
> crypto layer will happily memset some arbitrary memory for you amongst
> other things.
> 
> Anyways testing a fix now will post shortly.

I am aware that the space in question after aead_request is used internally 
by cryptoapi. My plan was to inflate this space by an extra sizeof(struct decrypt_req_ctx).

(*aead)->reqsize += sizeof(struct decrypt_req_ctx);

Somehow, I missed it.

But your piece of change look a better solution.
Thanks for submitting the fix.



> 
> Thanks,
> John


^ permalink raw reply

* RE: [net-next PATCH] tls: async support causes out-of-bounds access in crypto APIs
From: Vakul Garg @ 2018-09-15 12:13 UTC (permalink / raw)
  To: John Fastabend, davejwatson@fb.com
  Cc: doronrk@fb.com, netdev@vger.kernel.org,
	alexei.starovoitov@gmail.com, daniel@iogearbox.net,
	davem@davemloft.net
In-Reply-To: <20180914200146.6302.50472.stgit@john-Precision-Tower-5810>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of John Fastabend
> Sent: Saturday, September 15, 2018 1:32 AM
> To: Vakul Garg <vakul.garg@nxp.com>; davejwatson@fb.com
> Cc: doronrk@fb.com; netdev@vger.kernel.org;
> alexei.starovoitov@gmail.com; daniel@iogearbox.net;
> davem@davemloft.net
> Subject: [net-next PATCH] tls: async support causes out-of-bounds access in
> crypto APIs
> 
> When async support was added it needed to access the sk from the async
> callback to report errors up the stack. The patch tried to use space after the
> aead request struct by directly setting the reqsize field in aead_request. This
> is an internal field that should not be used outside the crypto APIs. It is used
> by the crypto code to define extra space for private structures used in the
> crypto context. Users of the API then use crypto_aead_reqsize() and add the
> returned amount of bytes to the end of the request memory allocation
> before posting the request to encrypt/decrypt APIs.
> 
> So this breaks (with general protection fault and KASAN error, if
> enabled) because the request sent to decrypt is shorter than required causing
> the crypto API out-of-bounds errors. Also it seems unlikely the sk is even valid
> by the time it gets to the callback because of memset in crypto layer.
> 
> Anyways, fix this by holding the sk in the skb->sk field when the callback is set
> up and because the skb is already passed through to the callback handler via
> void* we can access it in the handler. Then in the handler we need to be
> careful to NULL the pointer again before kfree_skb. I added comments on
> both the setup (in tls_do_decryption) and when we clear it from the crypto
> callback handler tls_decrypt_done(). After this selftests pass again and fixes
> KASAN errors/warnings.
> 
> Fixes: 94524d8fc965 ("net/tls: Add support for async decryption of tls
> records")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  include/net/tls.h |    4 ----
>  net/tls/tls_sw.c  |   39 +++++++++++++++++++++++----------------
>  2 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/tls.h b/include/net/tls.h index cd0a65b..8630d28
> 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -128,10 +128,6 @@ struct tls_sw_context_rx {
>  	bool async_notify;
>  };
> 
> -struct decrypt_req_ctx {
> -	struct sock *sk;
> -};
> -
>  struct tls_record_info {
>  	struct list_head list;
>  	u32 end_seq;
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index be4f2e9..cef69b6 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -122,25 +122,32 @@ static int skb_nsg(struct sk_buff *skb, int offset,
> int len)  static void tls_decrypt_done(struct crypto_async_request *req, int
> err)  {
>  	struct aead_request *aead_req = (struct aead_request *)req;
> -	struct decrypt_req_ctx *req_ctx =
> -			(struct decrypt_req_ctx *)(aead_req + 1);
> -
>  	struct scatterlist *sgout = aead_req->dst;
> -
> -	struct tls_context *tls_ctx = tls_get_ctx(req_ctx->sk);
> -	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> -	int pending = atomic_dec_return(&ctx->decrypt_pending);
> +	struct tls_sw_context_rx *ctx;
> +	struct tls_context *tls_ctx;
>  	struct scatterlist *sg;
> +	struct sk_buff *skb;
>  	unsigned int pages;
> +	int pending;
> +
> +	skb = (struct sk_buff *)req->data;
> +	tls_ctx = tls_get_ctx(skb->sk);
> +	ctx = tls_sw_ctx_rx(tls_ctx);
> +	pending = atomic_dec_return(&ctx->decrypt_pending);
> 
>  	/* Propagate if there was an err */
>  	if (err) {
>  		ctx->async_wait.err = err;
> -		tls_err_abort(req_ctx->sk, err);
> +		tls_err_abort(skb->sk, err);
>  	}
> 
> +	/* After using skb->sk to propagate sk through crypto async callback
> +	 * we need to NULL it again.
> +	 */
> +	skb->sk = NULL;
> +
>  	/* Release the skb, pages and memory allocated for crypto req */
> -	kfree_skb(req->data);
> +	kfree_skb(skb);
> 
>  	/* Skip the first S/G entry as it points to AAD */
>  	for_each_sg(sg_next(sgout), sg, UINT_MAX, pages) { @@ -175,11
> +182,13 @@ static int tls_do_decryption(struct sock *sk,
>  			       (u8 *)iv_recv);
> 
>  	if (async) {
> -		struct decrypt_req_ctx *req_ctx;
> -
> -		req_ctx = (struct decrypt_req_ctx *)(aead_req + 1);
> -		req_ctx->sk = sk;
> -
> +		/* Using skb->sk to push sk through to crypto async callback
> +		 * handler. This allows propagating errors up to the socket
> +		 * if needed. It _must_ be cleared in the async handler
> +		 * before kfree_skb is called. We _know_ skb->sk is NULL
> +		 * because it is a clone from strparser.
> +		 */
> +		skb->sk = sk;
>  		aead_request_set_callback(aead_req,
> 
> CRYPTO_TFM_REQ_MAY_BACKLOG,
>  					  tls_decrypt_done, skb);
> @@ -1455,8 +1464,6 @@ int tls_set_sw_offload(struct sock *sk, struct
> tls_context *ctx, int tx)
>  		goto free_aead;
> 
>  	if (sw_ctx_rx) {
> -		(*aead)->reqsize = sizeof(struct decrypt_req_ctx);
> -
>  		/* Set up strparser */
>  		memset(&cb, 0, sizeof(cb));
>  		cb.rcv_msg = tls_queue;

Reviewed-by: Vakul Garg <Vakul.garg@nxp.com>

^ permalink raw reply

* KMSAN: uninit-value in bond_start_xmit (2)
From: syzbot @ 2018-09-15 12:53 UTC (permalink / raw)
  To: andy, davem, j.vosburgh, linux-kernel, netdev, syzkaller-bugs,
	vfalico

Hello,

syzbot found the following crash on:

HEAD commit:    401f0ab26c43 kmsan: unpoison pages in do_read_cache_page()
git tree:       https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=17b808ae400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=6be230b6f0ad3490
dashboard link: https://syzkaller.appspot.com/bug?extid=e5be16aa39ad6e755391
compiler:       clang version 8.0.0 (trunk 339414)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124d16da400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=175f06be400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e5be16aa39ad6e755391@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KMSAN: uninit-value in bond_xmit_roundrobin  
drivers/net/bonding/bond_main.c:3858 [inline]
BUG: KMSAN: uninit-value in __bond_start_xmit  
drivers/net/bonding/bond_main.c:4136 [inline]
BUG: KMSAN: uninit-value in bond_start_xmit+0x1ab8/0x2b90  
drivers/net/bonding/bond_main.c:4170
CPU: 0 PID: 4319 Comm: syz-executor658 Not tainted 4.19.0-rc3+ #44
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x14b/0x190 lib/dump_stack.c:113
  kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:956
  __msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
  bond_xmit_roundrobin drivers/net/bonding/bond_main.c:3858 [inline]
  __bond_start_xmit drivers/net/bonding/bond_main.c:4136 [inline]
  bond_start_xmit+0x1ab8/0x2b90 drivers/net/bonding/bond_main.c:4170
  __netdev_start_xmit include/linux/netdevice.h:4287 [inline]
  netdev_start_xmit include/linux/netdevice.h:4296 [inline]
  xmit_one net/core/dev.c:3216 [inline]
  dev_hard_start_xmit+0x5df/0xc20 net/core/dev.c:3232
  __dev_queue_xmit+0x2f35/0x3ab0 net/core/dev.c:3802
  dev_queue_xmit+0x4b/0x60 net/core/dev.c:3835
  packet_snd net/packet/af_packet.c:2925 [inline]
  packet_sendmsg+0x80ff/0x8c60 net/packet/af_packet.c:2950
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg net/socket.c:631 [inline]
  ___sys_sendmsg+0xe70/0x1290 net/socket.c:2114
  __sys_sendmsg net/socket.c:2152 [inline]
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg+0x2a3/0x3d0 net/socket.c:2159
  __x64_sys_sendmsg+0x4a/0x70 net/socket.c:2159
  do_syscall_64+0xb8/0x100 arch/x86/entry/common.c:291
  entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x4410f9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffcc15c2468 EFLAGS: 00000217 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004410f9
RDX: 0000000000000000 RSI: 0000000020001940 RDI: 0000000000000004
RBP: 00000000006cc018 R08: 0000000000000100 R09: 0000000000000100
R10: 0000000000000100 R11: 0000000000000217 R12: 0000000000402060
R13: 00000000004020f0 R14: 0000000000000000 R15: 0000000000000000

Uninit was created at:
  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
  kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:181
  kmsan_kmalloc+0x98/0x100 mm/kmsan/kmsan_hooks.c:91
  kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan_hooks.c:100
  slab_post_alloc_hook mm/slab.h:446 [inline]
  slab_alloc_node mm/slub.c:2718 [inline]
  __kmalloc_node_track_caller+0x9e7/0x1160 mm/slub.c:4351
  __kmalloc_reserve net/core/skbuff.c:138 [inline]
  __alloc_skb+0x2f5/0x9e0 net/core/skbuff.c:206
  alloc_skb include/linux/skbuff.h:996 [inline]
  alloc_skb_with_frags+0x1d0/0xac0 net/core/skbuff.c:5276
  sock_alloc_send_pskb+0xb47/0x1170 net/core/sock.c:2082
  packet_alloc_skb net/packet/af_packet.c:2779 [inline]
  packet_snd net/packet/af_packet.c:2870 [inline]
  packet_sendmsg+0x6599/0x8c60 net/packet/af_packet.c:2950
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg net/socket.c:631 [inline]
  ___sys_sendmsg+0xe70/0x1290 net/socket.c:2114
  __sys_sendmsg net/socket.c:2152 [inline]
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg+0x2a3/0x3d0 net/socket.c:2159
  __x64_sys_sendmsg+0x4a/0x70 net/socket.c:2159
  do_syscall_64+0xb8/0x100 arch/x86/entry/common.c:291
  entry_SYSCALL_64_after_hwframe+0x63/0xe7
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH net] net: aquantia: memory corruption on jumbo frames
From: Igor Russkikh @ 2018-09-15 14:20 UTC (permalink / raw)
  To: netdev; +Cc: Igor Russkikh, Michael Rauch

From: Michael Rauch <michael@rauch.be>

This patch fixes skb_shared area, which will be corrupted
upon reception of 4K jumbo packets.

Originally build_skb usage purpose was to reuse page for skb to eliminate
needs of extra fragments. But that logic does not take into account that
skb_shared_info should be reserved at the end of skb data area.

In case packet data consumes all the page (4K), skb_shinfo location
overflows the page. As a consequence, __build_skb zeroed shinfo data above
the allocated page, corrupting next page.

The issue is rarely seen in real life because jumbo are normally larger
than 4K and that causes another code path to trigger.
But it 100% reproducible with simple scapy packet, like:

    sendp(IP(dst="192.168.100.3") / TCP(dport=443) \
          / Raw(RandString(size=(4096-40))), iface="enp1s0")

Fixes: 018423e90bee ("net: ethernet: aquantia: Add ring support code")

Reported-by: Michael Rauch <michael@rauch.be>
Tested-by: Michael Rauch <michael@rauch.be>
Signed-off-by: Michael Rauch <michael@rauch.be>
Tested-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 32 +++++++++++++-----------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index b5f1f62e8e25..d1e1a0ba8615 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -225,9 +225,10 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
 		}
 
 		/* for single fragment packets use build_skb() */
-		if (buff->is_eop) {
+		if (buff->is_eop &&
+		    buff->len <= AQ_CFG_RX_FRAME_MAX - AQ_SKB_ALIGN) {
 			skb = build_skb(page_address(buff->page),
-					buff->len + AQ_SKB_ALIGN);
+					AQ_CFG_RX_FRAME_MAX);
 			if (unlikely(!skb)) {
 				err = -ENOMEM;
 				goto err_exit;
@@ -247,18 +248,21 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
 					buff->len - ETH_HLEN,
 					SKB_TRUESIZE(buff->len - ETH_HLEN));
 
-			for (i = 1U, next_ = buff->next,
-			     buff_ = &self->buff_ring[next_]; true;
-			     next_ = buff_->next,
-			     buff_ = &self->buff_ring[next_], ++i) {
-				skb_add_rx_frag(skb, i, buff_->page, 0,
-						buff_->len,
-						SKB_TRUESIZE(buff->len -
-						ETH_HLEN));
-				buff_->is_cleaned = 1;
-
-				if (buff_->is_eop)
-					break;
+			if (!buff->is_eop) {
+				for (i = 1U, next_ = buff->next,
+				     buff_ = &self->buff_ring[next_];
+				     true; next_ = buff_->next,
+				     buff_ = &self->buff_ring[next_], ++i) {
+					skb_add_rx_frag(skb, i,
+							buff_->page, 0,
+							buff_->len,
+							SKB_TRUESIZE(buff->len -
+							ETH_HLEN));
+					buff_->is_cleaned = 1;
+
+					if (buff_->is_eop)
+						break;
+				}
 			}
 		}
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next v3 01/17] asm: simd context helper API
From: Andy Lutomirski @ 2018-09-15 19:58 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: kevin, Andrew Lutomirski, LKML, Netdev, David Miller,
	Greg Kroah-Hartman, Thomas Gleixner, Samuel Neves, linux-arch
In-Reply-To: <CAHmME9rMg_2eHBnfP6j9Y3O7-At90HF=ToTWeFL_Qc4nGURORg@mail.gmail.com>


> On Sep 13, 2018, at 6:52 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> 
>> On Thu, Sep 13, 2018 at 7:03 AM Kevin Easton <kevin@guarana.org> wrote:
>> Yes.  It's also how most get/put APIs already work in the kernel, eg
>> kref_get/put (mostly because they tend to be 'getting/putting' an
>> already-initialized object, though).
> 
> Right; in this case the object wouldn't be initialized yet, which
> might defeat the purpose, since one advantage of the & way you
> mentioned is _put modifies the context.
> 
> Andy - any opinions on this? The tl;dr is:
> 
> 1) what we have now:
> 
> simd_context_t simd_context = simd_get();
> for (item in items) {
>    do_something(item);
>    simd_context = simd_relax(simd_context);
> }
> simd_put();
> 
> 2) what kevin is proposing:
> 
> simd_context_t simd_context;
> 
> simd_get(&simd_context);
> for (item in items) {
>    do_something(item);
>    simd_relax(&simd_context);
> }
> simd_put(&simd_context);
> 
> I can see pros and cons of each approach.

I have no strong opinion, except that, if you paint the bikeshed “pass by value”, then simd_get and simd_relax should probably have warn_unused_result set.

^ permalink raw reply

* Re: [PATCH net-next v3 01/17] asm: simd context helper API
From: Jason A. Donenfeld @ 2018-09-15 20:01 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: kevin, Andrew Lutomirski, LKML, Netdev, David Miller,
	Greg Kroah-Hartman, Thomas Gleixner, Samuel Neves, linux-arch
In-Reply-To: <61B4DFD6-9BB5-4B81-BD0E-ACF112EA8AF7@amacapital.net>

On Sat, Sep 15, 2018 at 9:58 PM Andy Lutomirski <luto@amacapital.net> wrote:
> I have no strong opinion, except that, if you paint the bikeshed “pass by value”, then simd_get and simd_relax should probably have warn_unused_result set.

Good idea.

^ permalink raw reply

* Re: [PATCH net] net: aquantia: memory corruption on jumbo frames
From: Igor Russkikh @ 2018-09-15 14:26 UTC (permalink / raw)
  To: netdev, David S . Miller
In-Reply-To: <1ca2f3ebdd82d78b7ccbd86acd7ce9245316b42a.1536994993.git.igor.russkikh@aquantia.com>


> This patch fixes skb_shared area, which will be corrupted
> upon reception of 4K jumbo packets.
> 

Hi David, please discard this patch as it wrongly specifies the original issue reporter and patch author.
I'm fixing it now and will resend.
Sorry for this.

BR, Igor

^ permalink raw reply

* [PATCH v2 net] net: aquantia: memory corruption on jumbo frames
From: Igor Russkikh @ 2018-09-15 15:03 UTC (permalink / raw)
  To: David S . Miller; +Cc: Nikita Danilov, netdev, Igor Russkikh, Friedemann Gerold

From: Friedemann Gerold <f.gerold@b-c-s.de>

This patch fixes skb_shared area, which will be corrupted
upon reception of 4K jumbo packets.

Originally build_skb usage purpose was to reuse page for skb to eliminate
needs of extra fragments. But that logic does not take into account that
skb_shared_info should be reserved at the end of skb data area.

In case packet data consumes all the page (4K), skb_shinfo location
overflows the page. As a consequence, __build_skb zeroed shinfo data above
the allocated page, corrupting next page.

The issue is rarely seen in real life because jumbo are normally larger
than 4K and that causes another code path to trigger.
But it 100% reproducible with simple scapy packet, like:

    sendp(IP(dst="192.168.100.3") / TCP(dport=443) \
          / Raw(RandString(size=(4096-40))), iface="enp1s0")

Fixes: 018423e90bee ("net: ethernet: aquantia: Add ring support code")

Reported-by: Friedemann Gerold <f.gerold@b-c-s.de>
Reported-by: Michael Rauch <michael@rauch.be>
Signed-off-by: Friedemann Gerold <f.gerold@b-c-s.de>
Tested-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 32 +++++++++++++-----------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index b5f1f62e8e25..d1e1a0ba8615 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -225,9 +225,10 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
 		}
 
 		/* for single fragment packets use build_skb() */
-		if (buff->is_eop) {
+		if (buff->is_eop &&
+		    buff->len <= AQ_CFG_RX_FRAME_MAX - AQ_SKB_ALIGN) {
 			skb = build_skb(page_address(buff->page),
-					buff->len + AQ_SKB_ALIGN);
+					AQ_CFG_RX_FRAME_MAX);
 			if (unlikely(!skb)) {
 				err = -ENOMEM;
 				goto err_exit;
@@ -247,18 +248,21 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
 					buff->len - ETH_HLEN,
 					SKB_TRUESIZE(buff->len - ETH_HLEN));
 
-			for (i = 1U, next_ = buff->next,
-			     buff_ = &self->buff_ring[next_]; true;
-			     next_ = buff_->next,
-			     buff_ = &self->buff_ring[next_], ++i) {
-				skb_add_rx_frag(skb, i, buff_->page, 0,
-						buff_->len,
-						SKB_TRUESIZE(buff->len -
-						ETH_HLEN));
-				buff_->is_cleaned = 1;
-
-				if (buff_->is_eop)
-					break;
+			if (!buff->is_eop) {
+				for (i = 1U, next_ = buff->next,
+				     buff_ = &self->buff_ring[next_];
+				     true; next_ = buff_->next,
+				     buff_ = &self->buff_ring[next_], ++i) {
+					skb_add_rx_frag(skb, i,
+							buff_->page, 0,
+							buff_->len,
+							SKB_TRUESIZE(buff->len -
+							ETH_HLEN));
+					buff_->is_cleaned = 1;
+
+					if (buff_->is_eop)
+						break;
+				}
 			}
 		}
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 3/5] net: phy: mscc: remove unneeded parenthesis
From: Florian Fainelli @ 2018-09-15 20:52 UTC (permalink / raw)
  To: Quentin Schulz, davem, andrew
  Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <e3a1a9940e18c69ecf9aaecc3e2d9bf2a180a939.1536913944.git-series.quentin.schulz@bootlin.com>



On 9/14/2018 1:33 AM, Quentin Schulz wrote:
> The == operator precedes the || operator, so we can remove the
> parenthesis around (a == b) || (c == d).
> 
> The condition is rather explicit and short so removing the parenthesis
> definitely does not make it harder to read.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ 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