2019-05-06 21:27:56

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

Hi,

this patch series is an attempt to fix the mess, I've somehow managed to
introduce.

First patch in this series is defacto v5 of the previous 05/10 patch in the
series, but since the v4 of this 05/10 patch wasn't picked up by the
patchwork for some unknown reason, this patch wasn't applied with the other
9 patches in the series, so I'm resending it as a separate patch of this
fixup series again.

Second patch is a result of this rebase against net-next tree, where I was
checking again all current users of of_get_mac_address and found out, that
there's new one in DSA, so I've converted this user to the new ERR_PTR
encoded error value as well.

Third patch which was sent as v5 wasn't considered for merge, but I still
think, that we need to check for possible NULL value, thus current IS_ERR
check isn't sufficient and we need to use IS_ERR_OR_NULL instead.

Fourth patch fixes warning reported by kbuild test robot.

Cheers,

Petr

Petr Štetiar (4):
net: ethernet: support of_get_mac_address new ERR_PTR error
net: dsa: support of_get_mac_address new ERR_PTR error
staging: octeon-ethernet: Fix of_get_mac_address ERR_PTR check
net: usb: smsc: fix warning reported by kbuild test robot

drivers/net/ethernet/aeroflex/greth.c | 2 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 2 +-
drivers/net/ethernet/altera/altera_tse_main.c | 2 +-
drivers/net/ethernet/arc/emac_main.c | 2 +-
drivers/net/ethernet/aurora/nb8800.c | 2 +-
drivers/net/ethernet/broadcom/bcmsysport.c | 2 +-
drivers/net/ethernet/broadcom/bgmac-bcma.c | 2 +-
drivers/net/ethernet/broadcom/bgmac-platform.c | 2 +-
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 2 +-
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
drivers/net/ethernet/davicom/dm9000.c | 2 +-
drivers/net/ethernet/ethoc.c | 2 +-
drivers/net/ethernet/ezchip/nps_enet.c | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 2 +-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 2 +-
drivers/net/ethernet/freescale/fman/mac.c | 2 +-
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 2 +-
drivers/net/ethernet/freescale/gianfar.c | 2 +-
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 2 +-
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 2 +-
drivers/net/ethernet/lantiq_xrx200.c | 2 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
drivers/net/ethernet/marvell/mvneta.c | 2 +-
drivers/net/ethernet/marvell/pxa168_eth.c | 2 +-
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
drivers/net/ethernet/micrel/ks8851.c | 2 +-
drivers/net/ethernet/micrel/ks8851_mll.c | 2 +-
drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 2 +-
drivers/net/ethernet/qualcomm/qca_uart.c | 2 +-
drivers/net/ethernet/renesas/ravb_main.c | 2 +-
drivers/net/ethernet/renesas/sh_eth.c | 2 +-
drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c | 2 +-
drivers/net/ethernet/socionext/sni_ave.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
drivers/net/ethernet/ti/cpsw.c | 2 +-
drivers/net/ethernet/ti/netcp_core.c | 2 +-
drivers/net/ethernet/wiznet/w5100.c | 2 +-
drivers/net/ethernet/xilinx/ll_temac_main.c | 2 +-
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 2 +-
drivers/net/usb/smsc75xx.c | 2 +-
drivers/net/usb/smsc95xx.c | 2 +-
drivers/staging/octeon/ethernet.c | 2 +-
net/dsa/slave.c | 2 +-
net/ethernet/eth.c | 2 +-
49 files changed, 49 insertions(+), 49 deletions(-)

--
1.9.1


2019-05-06 21:28:12

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH net-next v2 3/4] staging: octeon-ethernet: Fix of_get_mac_address ERR_PTR check

Commit 284eb160681c ("staging: octeon-ethernet: support
of_get_mac_address new ERR_PTR error") has introduced checking for
ERR_PTR encoded error value from of_get_mac_address with IS_ERR macro,
which is not sufficient in this case, as the mac variable is set to NULL
initialy and if the kernel is compiled without DT support this NULL
would get passed to IS_ERR, which would lead to the wrong decision and
would pass that NULL pointer and invalid MAC address further.

Fixes: 284eb160681c ("staging: octeon-ethernet: support of_get_mac_address new ERR_PTR error")
Signed-off-by: Petr Štetiar <[email protected]>
---
drivers/staging/octeon/ethernet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 2b03018..8847a11c2 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -421,7 +421,7 @@ int cvm_oct_common_init(struct net_device *dev)
if (priv->of_node)
mac = of_get_mac_address(priv->of_node);

- if (!IS_ERR(mac))
+ if (!IS_ERR_OR_NULL(mac))
ether_addr_copy(dev->dev_addr, mac);
else
eth_hw_addr_random(dev);
--
1.9.1

2019-05-07 07:22:45

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

On Mon, May 06, 2019 at 11:24:43PM +0200, Petr Štetiar wrote:
> Hi,
>
> this patch series is an attempt to fix the mess, I've somehow managed to
> introduce.
>
> First patch in this series is defacto v5 of the previous 05/10 patch in the
> series, but since the v4 of this 05/10 patch wasn't picked up by the
> patchwork for some unknown reason, this patch wasn't applied with the other
> 9 patches in the series, so I'm resending it as a separate patch of this
> fixup series again.

I feel sort of ridiculous asking this over and over... Maybe your spam
filter is eating my emails?

This bug was introduced in https://patchwork.ozlabs.org/patch/1094916/
"[v4,01/10] of_net: add NVMEM support to of_get_mac_address" but it
looks like no one applied it.

You're acting as if it *was* applied but you refuse to answer my
question who applied it and which to which tree so I can figure out what
went wrong.

I only see comments from last Friday that it shouldn't be applied... I
also told you on Friday in a different thread that that patch shouldn't
be applied. Breaking git bisect is a bug, and we never do that. I'm
just very confused right now... What I'm trying to do is figure out in
my head how this process failed so we can do better next time.

regards,
dan carpenter

2019-05-07 07:42:38

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

On Tue, May 07, 2019 at 10:19:14AM +0300, Dan Carpenter wrote:
> On Mon, May 06, 2019 at 11:24:43PM +0200, Petr Štetiar wrote:
> > Hi,
> >
> > this patch series is an attempt to fix the mess, I've somehow managed to
> > introduce.
> >
> > First patch in this series is defacto v5 of the previous 05/10 patch in the
> > series, but since the v4 of this 05/10 patch wasn't picked up by the
> > patchwork for some unknown reason, this patch wasn't applied with the other
> > 9 patches in the series, so I'm resending it as a separate patch of this
> > fixup series again.
>
> I feel sort of ridiculous asking this over and over... Maybe your spam
> filter is eating my emails?
>
> This bug was introduced in https://patchwork.ozlabs.org/patch/1094916/
> "[v4,01/10] of_net: add NVMEM support to of_get_mac_address" but it
> looks like no one applied it.
>
> You're acting as if it *was* applied but you refuse to answer my
> question who applied it and which to which tree so I can figure out what
> went wrong.
>
> I only see comments from last Friday that it shouldn't be applied... I
> also told you on Friday in a different thread that that patch shouldn't
> be applied. Breaking git bisect is a bug, and we never do that. I'm
> just very confused right now... What I'm trying to do is figure out in
> my head how this process failed so we can do better next time.

Just to resend this, so that it hopefully does _not_ get stuck in a spam
filter.

Petr, please address Dan's comments, do not ignore them.

greg k-h

2019-05-07 08:40:33

by Petr Štetiar

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

Dan Carpenter <[email protected]> [2019-05-07 10:19:14]:

Hi,

> On Mon, May 06, 2019 at 11:24:43PM +0200, Petr Štetiar wrote:
> >
> > this patch series is an attempt to fix the mess, I've somehow managed to
> > introduce.
> >
> > First patch in this series is defacto v5 of the previous 05/10 patch in the
> > series, but since the v4 of this 05/10 patch wasn't picked up by the
> > patchwork for some unknown reason, this patch wasn't applied with the other
> > 9 patches in the series, so I'm resending it as a separate patch of this
> > fixup series again.
>
> I feel sort of ridiculous asking this over and over... Maybe your spam
> filter is eating my emails?

nope, I've read your email, that's the only reason I've sent out this v2 which
added Fixes: tag you've suggested in your email.

> This bug was introduced in https://patchwork.ozlabs.org/patch/1094916/
> "[v4,01/10] of_net: add NVMEM support to of_get_mac_address" but it
> looks like no one applied it.

this patch series is against net-next, and I've added Fixes: tag as you've
requested in this v2 series[1] (which expands to commit[2] in net-next):

Fixes: d01f449c008a ("of_net: add NVMEM support to of_get_mac_address")

> You're acting as if it *was* applied but you refuse to answer my
> question who applied it and which to which tree so I can figure out what
> went wrong.

it was applied[2] to David's net-next tree, but unfortunately partialy, just 9
patches out of 10, as the patch 05/10 in that series (which is patch 1/4 in
this series) never reached netdev mailing list and patchwork, probably because
of some netdev mailing list software and/or patchwork hiccup, very likely due
to the long list of recipients in that patch and as I'm not subscribed to the
netdev (due to the high traffic) I'm probably treaten somehow differently.

So to sum it up, I've simply thought, that it was enough to send out v2 with
that Fixes: tag and considered it done.

> I only see comments from last Friday that it shouldn't be applied...

I'm sorry, but which comments do you mean exactly? Those about the
`nvmem-mac-address` DT (sysfs) entry? If that is the case, from my point of
view, I've provided reasonable arguments and nobody told me, that I'm wrong
with my reasoning or NACKed this explicitly, so David probably considered my
arguments good enough and merged it as it is? I don't have any other
explanation.

> I also told you on Friday in a different thread that that patch shouldn't be
> applied. Breaking git bisect is a bug, and we never do that.

Yes, and I agree with you, but I've simply thought, that if any of the
maintainers who previously reviewed the series didn't objected about this,
that they're possibly going to squash those patches by themselves during the
merging process or that they're going to tell me to do so and I would address
this in the latest interation of the patchset before merge.

Anyway, is there any possibility how to fix that now?

> I'm just very confused right now. What I'm trying to do is figure out in
> my head how this process failed so we can do better next time.

I'm just occasional contributor, so I'm sorry, but I can hardly provide any
input.

1. https://patchwork.ozlabs.org/patch/1096054/
2. https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=d01f449c008a3f41fa44c603e28a7452ab8f8e68

Cheers,

Petr

2019-05-07 09:03:02

by Petr Štetiar

[permalink] [raw]
Subject: netdev patchwork issues [Was: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes]

Petr Štetiar <[email protected]> [2019-05-07 10:39:18]:

[adding Jeremy to the Cc: loop]

> it was applied[2] to David's net-next tree, but unfortunately partialy, just 9
> patches out of 10, as the patch 05/10 in that series (which is patch 1/4 in
> this series) never reached netdev mailing list and patchwork, probably because
> of some netdev mailing list software and/or patchwork hiccup, very likely due
> to the long list of recipients in that patch and as I'm not subscribed to the
> netdev (due to the high traffic) I'm probably treaten somehow differently.

For the record, I've following in my ~/.gitconfig:

[sendemail.linux]
tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"
cccmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nom"

and I've sent the patches with the following command:

git send-email \
--to [email protected] \
--to 'David S. Miller <[email protected]>' \
--cc 'Andrew Lunn <[email protected]>' \
--cc 'Florian Fainelli <[email protected]>' \
--cc 'Heiner Kallweit <[email protected]>' \
--cc 'Frank Rowand <[email protected]>' \
--cc '[email protected]' \
--cc '[email protected]' \
--cc 'Greg Kroah-Hartman <[email protected]>' \
--cc 'Maxime Ripard <[email protected]>' \
--identity linux tmp/nvmem-fix-v2/000*

which resulted just in the following 4 bounces:

* [email protected] (no such recipient)
* [email protected] (no such recipient)
* [email protected] (no such recipient)

Your mail to 'linux-arm-kernel' with the subject

[PATCH net-next v2 1/4] net: ethernet: support of_get_mac_address new ERR_PTR error

Is being held until the list moderator can review it for approval. The reason
it is being held:

Too many recipients to the message

So maybe netdev have similar moderation stuff enabled, but doesn't send out
this notices back? I've "fixed" the issue with the following workaround:

git send-email \
--to [email protected] \
--in-reply-to '<[email protected]>' \
tmp/nvmem-fix-v2/0001-net-ethernet-support-of_get_mac_address-new-ERR_PTR-.patch

That is, just using netdev as the sole recipient and then the patch has
appeared in the patchwork and in the mailing list archive as well.

-- ynezz

2019-05-07 09:09:06

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

Hi Petr,

On Mon, May 6, 2019 at 11:25 PM Petr Štetiar <[email protected]> wrote:
> this patch series is an attempt to fix the mess, I've somehow managed to
> introduce.
>
> First patch in this series is defacto v5 of the previous 05/10 patch in the
> series, but since the v4 of this 05/10 patch wasn't picked up by the
> patchwork for some unknown reason, this patch wasn't applied with the other
> 9 patches in the series, so I'm resending it as a separate patch of this
> fixup series again.
>
> Second patch is a result of this rebase against net-next tree, where I was
> checking again all current users of of_get_mac_address and found out, that
> there's new one in DSA, so I've converted this user to the new ERR_PTR
> encoded error value as well.
>
> Third patch which was sent as v5 wasn't considered for merge, but I still
> think, that we need to check for possible NULL value, thus current IS_ERR
> check isn't sufficient and we need to use IS_ERR_OR_NULL instead.
>
> Fourth patch fixes warning reported by kbuild test robot.
>
> Cheers,
>
> Petr
>
> Petr Štetiar (4):
> net: ethernet: support of_get_mac_address new ERR_PTR error

I didn't receive the patch through email, but patchwork does have it:
https://patchwork.ozlabs.org/patch/1096054/

This fixes the crash ("Unable to handle kernel paging request atvirtual
address fffffffe") I'm seeing with sh_eth on r8a7791/koelsch, so

Tested-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-05-07 09:48:41

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

Oh crap. You did add a Fixes tag. My bad.

I should have been more clear/pro-active on Friday and we could have
avoided this... Next time.

regards,
dan carpenter

2019-05-07 12:16:10

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

On 07.05.2019 00:25, Petr ?tetiar wrote:
> Hi,
>
> this patch series is an attempt to fix the mess, I've somehow managed to
> introduce.
>
> First patch in this series is defacto v5 of the previous 05/10 patch in the
> series, but since the v4 of this 05/10 patch wasn't picked up by the
> patchwork for some unknown reason, this patch wasn't applied with the other
> 9 patches in the series, so I'm resending it as a separate patch of this
> fixup series again.
>
> Second patch is a result of this rebase against net-next tree, where I was
> checking again all current users of of_get_mac_address and found out, that
> there's new one in DSA, so I've converted this user to the new ERR_PTR
> encoded error value as well.
>
> Third patch which was sent as v5 wasn't considered for merge, but I still
> think, that we need to check for possible NULL value, thus current IS_ERR
> check isn't sufficient and we need to use IS_ERR_OR_NULL instead.
>
> Fourth patch fixes warning reported by kbuild test robot.
>
> Cheers,
>
> Petr
>
> Petr ?tetiar (4):
> net: ethernet: support of_get_mac_address new ERR_PTR error
> net: dsa: support of_get_mac_address new ERR_PTR error
> staging: octeon-ethernet: Fix of_get_mac_address ERR_PTR check
> net: usb: smsc: fix warning reported by kbuild test robot

> drivers/net/ethernet/freescale/fec_main.c | 2 +-

This fixes netboot on imx (probably all of them).

Tested-by: Leonard Crestez <[email protected]>

But shouldn't "support of_get_mac_address new ERR_PTR error" somehow be
reordered so that it's done before allowing non-null errors from
of_get_mac_address?

Otherwise it will break bisect for many people.

--
Regards,
Leonard

2019-05-07 19:24:15

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] of_get_mac_address ERR_PTR fixes

From: Petr ?tetiar <[email protected]>
Date: Mon, 6 May 2019 23:24:43 +0200

> this patch series is an attempt to fix the mess, I've somehow managed to
> introduce.
>
> First patch in this series is defacto v5 of the previous 05/10 patch in the
> series, but since the v4 of this 05/10 patch wasn't picked up by the
> patchwork for some unknown reason, this patch wasn't applied with the other
> 9 patches in the series, so I'm resending it as a separate patch of this
> fixup series again.
>
> Second patch is a result of this rebase against net-next tree, where I was
> checking again all current users of of_get_mac_address and found out, that
> there's new one in DSA, so I've converted this user to the new ERR_PTR
> encoded error value as well.
>
> Third patch which was sent as v5 wasn't considered for merge, but I still
> think, that we need to check for possible NULL value, thus current IS_ERR
> check isn't sufficient and we need to use IS_ERR_OR_NULL instead.
>
> Fourth patch fixes warning reported by kbuild test robot.

Series applied, thanks.