* [PATCH 01/25] arch: powerpc: mpc85xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:24 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 02/25] board: gdsys: a38x: " Vladimir Oltean
` (23 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
index 3c4eb1a7eba9..1f6f55707321 100644
--- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c
+++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
@@ -444,7 +444,7 @@ int fec_initialize(struct bd_info *bis)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = bb_miiphy_read;
mdiodev->write = bb_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 01/25] arch: powerpc: mpc85xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 01/25] arch: powerpc: mpc85xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation Vladimir Oltean
@ 2021-09-28 13:24 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:24 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> index 3c4eb1a7eba9..1f6f55707321 100644
> --- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> +++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> @@ -444,7 +444,7 @@ int fec_initialize(struct bd_info *bis)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = bb_miiphy_read;
> mdiodev->write = bb_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 02/25] board: gdsys: a38x: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
2021-09-27 11:21 ` [PATCH 01/25] arch: powerpc: mpc85xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:24 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 03/25] net: armada100_fec: " Vladimir Oltean
` (22 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
board/gdsys/a38x/ihs_phys.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/board/gdsys/a38x/ihs_phys.c b/board/gdsys/a38x/ihs_phys.c
index c23d15092144..e09c0006b76f 100644
--- a/board/gdsys/a38x/ihs_phys.c
+++ b/board/gdsys/a38x/ihs_phys.c
@@ -110,9 +110,7 @@ int register_miiphy_bus(uint k, struct mii_dev **bus)
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name,
- name,
- MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, name, MDIO_NAME_LEN);
mdiodev->read = bb_miiphy_read;
mdiodev->write = bb_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 02/25] board: gdsys: a38x: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 02/25] board: gdsys: a38x: " Vladimir Oltean
@ 2021-09-28 13:24 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:24 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> board/gdsys/a38x/ihs_phys.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/board/gdsys/a38x/ihs_phys.c b/board/gdsys/a38x/ihs_phys.c
> index c23d15092144..e09c0006b76f 100644
> --- a/board/gdsys/a38x/ihs_phys.c
> +++ b/board/gdsys/a38x/ihs_phys.c
> @@ -110,9 +110,7 @@ int register_miiphy_bus(uint k, struct mii_dev **bus)
>
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name,
> - name,
> - MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, name, MDIO_NAME_LEN);
> mdiodev->read = bb_miiphy_read;
> mdiodev->write = bb_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 03/25] net: armada100_fec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
2021-09-27 11:21 ` [PATCH 01/25] arch: powerpc: mpc85xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation Vladimir Oltean
2021-09-27 11:21 ` [PATCH 02/25] board: gdsys: a38x: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:31 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 04/25] net: at91_emac: " Vladimir Oltean
` (21 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/armada100_fec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/armada100_fec.c b/drivers/net/armada100_fec.c
index 018891e173c3..5d4b90c6ba72 100644
--- a/drivers/net/armada100_fec.c
+++ b/drivers/net/armada100_fec.c
@@ -717,7 +717,7 @@ int armada100_fec_register(unsigned long base_addr)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = smi_reg_read;
mdiodev->write = smi_reg_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 03/25] net: armada100_fec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 03/25] net: armada100_fec: " Vladimir Oltean
@ 2021-09-28 13:31 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:31 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/armada100_fec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/armada100_fec.c b/drivers/net/armada100_fec.c
> index 018891e173c3..5d4b90c6ba72 100644
> --- a/drivers/net/armada100_fec.c
> +++ b/drivers/net/armada100_fec.c
> @@ -717,7 +717,7 @@ int armada100_fec_register(unsigned long base_addr)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = smi_reg_read;
> mdiodev->write = smi_reg_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 04/25] net: at91_emac: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (2 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 03/25] net: armada100_fec: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:31 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 05/25] net: bcm-sf2: " Vladimir Oltean
` (20 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/at91_emac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/at91_emac.c b/drivers/net/at91_emac.c
index e40b94ad892d..b4581d8c9320 100644
--- a/drivers/net/at91_emac.c
+++ b/drivers/net/at91_emac.c
@@ -507,7 +507,7 @@ int at91emac_register(struct bd_info *bis, unsigned long iobase)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = at91emac_mii_read;
mdiodev->write = at91emac_mii_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 04/25] net: at91_emac: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 04/25] net: at91_emac: " Vladimir Oltean
@ 2021-09-28 13:31 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:31 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/at91_emac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/at91_emac.c b/drivers/net/at91_emac.c
> index e40b94ad892d..b4581d8c9320 100644
> --- a/drivers/net/at91_emac.c
> +++ b/drivers/net/at91_emac.c
> @@ -507,7 +507,7 @@ int at91emac_register(struct bd_info *bis, unsigned long iobase)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = at91emac_mii_read;
> mdiodev->write = at91emac_mii_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 05/25] net: bcm-sf2: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (3 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 04/25] net: at91_emac: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:31 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 06/25] net: eepro100: " Vladimir Oltean
` (19 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/bcm-sf2-eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bcm-sf2-eth.c b/drivers/net/bcm-sf2-eth.c
index c862c141461c..88dc3ab38466 100644
--- a/drivers/net/bcm-sf2-eth.c
+++ b/drivers/net/bcm-sf2-eth.c
@@ -250,7 +250,7 @@ int bcm_sf2_eth_register(struct bd_info *bis, u8 dev_num)
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = eth->miiphy_read;
mdiodev->write = eth->miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 05/25] net: bcm-sf2: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 05/25] net: bcm-sf2: " Vladimir Oltean
@ 2021-09-28 13:31 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:31 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/bcm-sf2-eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bcm-sf2-eth.c b/drivers/net/bcm-sf2-eth.c
> index c862c141461c..88dc3ab38466 100644
> --- a/drivers/net/bcm-sf2-eth.c
> +++ b/drivers/net/bcm-sf2-eth.c
> @@ -250,7 +250,7 @@ int bcm_sf2_eth_register(struct bd_info *bis, u8 dev_num)
>
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = eth->miiphy_read;
> mdiodev->write = eth->miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 06/25] net: eepro100: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (4 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 05/25] net: bcm-sf2: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:31 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 07/25] net: ep93xx: " Vladimir Oltean
` (18 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/eepro100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/eepro100.c b/drivers/net/eepro100.c
index 934b881219e9..935cd9c99cef 100644
--- a/drivers/net/eepro100.c
+++ b/drivers/net/eepro100.c
@@ -493,7 +493,7 @@ static int eepro100_initialize_mii(struct eepro100_priv *priv)
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, priv->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, priv->name, MDIO_NAME_LEN);
mdiodev->read = eepro100_miiphy_read;
mdiodev->write = eepro100_miiphy_write;
mdiodev->priv = priv;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 06/25] net: eepro100: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 06/25] net: eepro100: " Vladimir Oltean
@ 2021-09-28 13:31 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:31 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/eepro100.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/eepro100.c b/drivers/net/eepro100.c
> index 934b881219e9..935cd9c99cef 100644
> --- a/drivers/net/eepro100.c
> +++ b/drivers/net/eepro100.c
> @@ -493,7 +493,7 @@ static int eepro100_initialize_mii(struct eepro100_priv *priv)
> if (!mdiodev)
> return -ENOMEM;
>
> - strncpy(mdiodev->name, priv->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, priv->name, MDIO_NAME_LEN);
> mdiodev->read = eepro100_miiphy_read;
> mdiodev->write = eepro100_miiphy_write;
> mdiodev->priv = priv;
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 07/25] net: ep93xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (5 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 06/25] net: eepro100: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 08/25] net: enetc: ensure imdio.name " Vladimir Oltean
` (17 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ep93xx_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ep93xx_eth.c b/drivers/net/ep93xx_eth.c
index 0218349b0450..9f8df7de0609 100644
--- a/drivers/net/ep93xx_eth.c
+++ b/drivers/net/ep93xx_eth.c
@@ -427,7 +427,7 @@ int ep93xx_miiphy_initialize(struct bd_info * const bd)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, "ep93xx_eth0", MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, "ep93xx_eth0", MDIO_NAME_LEN);
mdiodev->read = ep93xx_miiphy_read;
mdiodev->write = ep93xx_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 07/25] net: ep93xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 07/25] net: ep93xx: " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ep93xx_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ep93xx_eth.c b/drivers/net/ep93xx_eth.c
> index 0218349b0450..9f8df7de0609 100644
> --- a/drivers/net/ep93xx_eth.c
> +++ b/drivers/net/ep93xx_eth.c
> @@ -427,7 +427,7 @@ int ep93xx_miiphy_initialize(struct bd_info * const bd)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, "ep93xx_eth0", MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, "ep93xx_eth0", MDIO_NAME_LEN);
> mdiodev->read = ep93xx_miiphy_read;
> mdiodev->write = ep93xx_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 08/25] net: enetc: ensure imdio.name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (6 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 07/25] net: ep93xx: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 09/25] net: mcdmafec: ensure bus->name " Vladimir Oltean
` (16 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/fsl_enetc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 566cdc7e546a..b7e2c1f0880c 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -270,7 +270,7 @@ static void enetc_start_pcs(struct udevice *dev)
priv->imdio.read = enetc_mdio_read;
priv->imdio.write = enetc_mdio_write;
priv->imdio.priv = priv->port_regs + ENETC_PM_IMDIO_BASE;
- strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
+ strlcpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
if (!miiphy_get_dev_by_name(priv->imdio.name))
mdio_register(&priv->imdio);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 08/25] net: enetc: ensure imdio.name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 08/25] net: enetc: ensure imdio.name " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/fsl_enetc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
> index 566cdc7e546a..b7e2c1f0880c 100644
> --- a/drivers/net/fsl_enetc.c
> +++ b/drivers/net/fsl_enetc.c
> @@ -270,7 +270,7 @@ static void enetc_start_pcs(struct udevice *dev)
> priv->imdio.read = enetc_mdio_read;
> priv->imdio.write = enetc_mdio_write;
> priv->imdio.priv = priv->port_regs + ENETC_PM_IMDIO_BASE;
> - strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
> + strlcpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
> if (!miiphy_get_dev_by_name(priv->imdio.name))
> mdio_register(&priv->imdio);
> }
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 09/25] net: mcdmafec: ensure bus->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (7 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 08/25] net: enetc: ensure imdio.name " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 10/25] net: ftmac110: ensure mdiodev->name " Vladimir Oltean
` (15 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/fsl_mcdmafec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c
index c20aef4ab28d..e103f79305e7 100644
--- a/drivers/net/fsl_mcdmafec.c
+++ b/drivers/net/fsl_mcdmafec.c
@@ -541,7 +541,7 @@ static int mcdmafec_probe(struct udevice *dev)
info->bus = mdio_alloc();
if (!info->bus)
return -ENOMEM;
- strncpy(info->bus->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(info->bus->name, dev->name, MDIO_NAME_LEN);
info->bus->read = mcffec_miiphy_read;
info->bus->write = mcffec_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 09/25] net: mcdmafec: ensure bus->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 09/25] net: mcdmafec: ensure bus->name " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/fsl_mcdmafec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c
> index c20aef4ab28d..e103f79305e7 100644
> --- a/drivers/net/fsl_mcdmafec.c
> +++ b/drivers/net/fsl_mcdmafec.c
> @@ -541,7 +541,7 @@ static int mcdmafec_probe(struct udevice *dev)
> info->bus = mdio_alloc();
> if (!info->bus)
> return -ENOMEM;
> - strncpy(info->bus->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(info->bus->name, dev->name, MDIO_NAME_LEN);
> info->bus->read = mcffec_miiphy_read;
> info->bus->write = mcffec_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 10/25] net: ftmac110: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (8 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 09/25] net: mcdmafec: ensure bus->name " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 11/25] net: lpc32xx: " Vladimir Oltean
` (14 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ftmac110.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ftmac110.c b/drivers/net/ftmac110.c
index 265d813c4f89..7e54d4642ddf 100644
--- a/drivers/net/ftmac110.c
+++ b/drivers/net/ftmac110.c
@@ -476,7 +476,7 @@ int ftmac110_initialize(struct bd_info *bis)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = ftmac110_mdio_read;
mdiodev->write = ftmac110_mdio_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 10/25] net: ftmac110: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 10/25] net: ftmac110: ensure mdiodev->name " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ftmac110.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ftmac110.c b/drivers/net/ftmac110.c
> index 265d813c4f89..7e54d4642ddf 100644
> --- a/drivers/net/ftmac110.c
> +++ b/drivers/net/ftmac110.c
> @@ -476,7 +476,7 @@ int ftmac110_initialize(struct bd_info *bis)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = ftmac110_mdio_read;
> mdiodev->write = ftmac110_mdio_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 11/25] net: lpc32xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (9 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 10/25] net: ftmac110: ensure mdiodev->name " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 12/25] net: macb: " Vladimir Oltean
` (13 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/lpc32xx_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
index 3f281a515c6a..1a5734343935 100644
--- a/drivers/net/lpc32xx_eth.c
+++ b/drivers/net/lpc32xx_eth.c
@@ -638,7 +638,7 @@ int lpc32xx_eth_initialize(struct bd_info *bis)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = mii_reg_read;
mdiodev->write = mii_reg_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 11/25] net: lpc32xx: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 11/25] net: lpc32xx: " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/lpc32xx_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
> index 3f281a515c6a..1a5734343935 100644
> --- a/drivers/net/lpc32xx_eth.c
> +++ b/drivers/net/lpc32xx_eth.c
> @@ -638,7 +638,7 @@ int lpc32xx_eth_initialize(struct bd_info *bis)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = mii_reg_read;
> mdiodev->write = mii_reg_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 12/25] net: macb: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (10 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 11/25] net: lpc32xx: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 13/25] net: mpc8xx_fec: " Vladimir Oltean
` (12 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/macb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 57ea45e2dc7f..8151104acfc0 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -1245,7 +1245,7 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, netdev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, netdev->name, MDIO_NAME_LEN);
mdiodev->read = macb_miiphy_read;
mdiodev->write = macb_miiphy_write;
@@ -1403,7 +1403,7 @@ static int macb_eth_probe(struct udevice *dev)
macb->bus = mdio_alloc();
if (!macb->bus)
return -ENOMEM;
- strncpy(macb->bus->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(macb->bus->name, dev->name, MDIO_NAME_LEN);
macb->bus->read = macb_miiphy_read;
macb->bus->write = macb_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 12/25] net: macb: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 12/25] net: macb: " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/macb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 57ea45e2dc7f..8151104acfc0 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -1245,7 +1245,7 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, netdev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, netdev->name, MDIO_NAME_LEN);
> mdiodev->read = macb_miiphy_read;
> mdiodev->write = macb_miiphy_write;
>
> @@ -1403,7 +1403,7 @@ static int macb_eth_probe(struct udevice *dev)
> macb->bus = mdio_alloc();
> if (!macb->bus)
> return -ENOMEM;
> - strncpy(macb->bus->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(macb->bus->name, dev->name, MDIO_NAME_LEN);
> macb->bus->read = macb_miiphy_read;
> macb->bus->write = macb_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 13/25] net: mpc8xx_fec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (11 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 12/25] net: macb: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:32 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 14/25] net: dsa: felix: ensure mii_bus->name " Vladimir Oltean
` (11 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/mpc8xx_fec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mpc8xx_fec.c b/drivers/net/mpc8xx_fec.c
index 282c2599d3c4..4eb826028111 100644
--- a/drivers/net/mpc8xx_fec.c
+++ b/drivers/net/mpc8xx_fec.c
@@ -160,7 +160,7 @@ int fec_initialize(struct bd_info *bis)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = fec8xx_miiphy_read;
mdiodev->write = fec8xx_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 13/25] net: mpc8xx_fec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 13/25] net: mpc8xx_fec: " Vladimir Oltean
@ 2021-09-28 13:32 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:32 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/mpc8xx_fec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/mpc8xx_fec.c b/drivers/net/mpc8xx_fec.c
> index 282c2599d3c4..4eb826028111 100644
> --- a/drivers/net/mpc8xx_fec.c
> +++ b/drivers/net/mpc8xx_fec.c
> @@ -160,7 +160,7 @@ int fec_initialize(struct bd_info *bis)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = fec8xx_miiphy_read;
> mdiodev->write = fec8xx_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 14/25] net: dsa: felix: ensure mii_bus->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (12 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 13/25] net: mpc8xx_fec: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 15/25] net: mvgbe: ensure mdiodev->name " Vladimir Oltean
` (10 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/mscc_eswitch/felix_switch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
index 6aa79784460d..4c2e57755967 100644
--- a/drivers/net/mscc_eswitch/felix_switch.c
+++ b/drivers/net/mscc_eswitch/felix_switch.c
@@ -258,7 +258,7 @@ static void felix_init(struct udevice *dev)
priv->imdio.read = felix_mdio_read;
priv->imdio.write = felix_mdio_write;
priv->imdio.priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
- strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
+ strlcpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
/* set up CPU port */
out_le32(base + FELIX_QSYS_SYSTEM_EXT_CPU_CFG,
@@ -303,7 +303,7 @@ static int felix_probe(struct udevice *dev)
mii_bus->read = felix_mdio_read;
mii_bus->write = felix_mdio_write;
mii_bus->priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
- strncpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
mdio_register(mii_bus);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 14/25] net: dsa: felix: ensure mii_bus->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 14/25] net: dsa: felix: ensure mii_bus->name " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/mscc_eswitch/felix_switch.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
> index 6aa79784460d..4c2e57755967 100644
> --- a/drivers/net/mscc_eswitch/felix_switch.c
> +++ b/drivers/net/mscc_eswitch/felix_switch.c
> @@ -258,7 +258,7 @@ static void felix_init(struct udevice *dev)
> priv->imdio.read = felix_mdio_read;
> priv->imdio.write = felix_mdio_write;
> priv->imdio.priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
> - strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
> + strlcpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
>
> /* set up CPU port */
> out_le32(base + FELIX_QSYS_SYSTEM_EXT_CPU_CFG,
> @@ -303,7 +303,7 @@ static int felix_probe(struct udevice *dev)
> mii_bus->read = felix_mdio_read;
> mii_bus->write = felix_mdio_write;
> mii_bus->priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
> - strncpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
> mdio_register(mii_bus);
> }
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 15/25] net: mvgbe: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (13 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 14/25] net: dsa: felix: ensure mii_bus->name " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 16/25] net: sh_eth: " Vladimir Oltean
` (9 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/mvgbe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c
index ce5b8eed64b4..954bf86121a4 100644
--- a/drivers/net/mvgbe.c
+++ b/drivers/net/mvgbe.c
@@ -883,7 +883,7 @@ int mvgbe_initialize(struct bd_info *bis)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = smi_reg_read;
mdiodev->write = smi_reg_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 15/25] net: mvgbe: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 15/25] net: mvgbe: ensure mdiodev->name " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/mvgbe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c
> index ce5b8eed64b4..954bf86121a4 100644
> --- a/drivers/net/mvgbe.c
> +++ b/drivers/net/mvgbe.c
> @@ -883,7 +883,7 @@ int mvgbe_initialize(struct bd_info *bis)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = smi_reg_read;
> mdiodev->write = smi_reg_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 16/25] net: sh_eth: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (14 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 15/25] net: mvgbe: ensure mdiodev->name " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 17/25] net: smc911x: " Vladimir Oltean
` (8 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/sh_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
index 3143a5813a6d..4055f07b2feb 100644
--- a/drivers/net/sh_eth.c
+++ b/drivers/net/sh_eth.c
@@ -657,7 +657,7 @@ int sh_eth_initialize(struct bd_info *bd)
mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = bb_miiphy_read;
mdiodev->write = bb_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 16/25] net: sh_eth: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 16/25] net: sh_eth: " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/sh_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
> index 3143a5813a6d..4055f07b2feb 100644
> --- a/drivers/net/sh_eth.c
> +++ b/drivers/net/sh_eth.c
> @@ -657,7 +657,7 @@ int sh_eth_initialize(struct bd_info *bd)
> mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = bb_miiphy_read;
> mdiodev->write = bb_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 17/25] net: smc911x: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (15 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 16/25] net: sh_eth: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 18/25] net: davinci_emac: " Vladimir Oltean
` (7 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/smc911x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 8f420261fa8d..5d9a73f23d75 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -425,7 +425,7 @@ static int smc911x_initialize_mii(struct smc911x_priv *priv)
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, priv->dev.name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, priv->dev.name, MDIO_NAME_LEN);
mdiodev->read = smc911x_miiphy_read;
mdiodev->write = smc911x_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 17/25] net: smc911x: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 17/25] net: smc911x: " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/smc911x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 8f420261fa8d..5d9a73f23d75 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -425,7 +425,7 @@ static int smc911x_initialize_mii(struct smc911x_priv *priv)
> if (!mdiodev)
> return -ENOMEM;
>
> - strncpy(mdiodev->name, priv->dev.name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, priv->dev.name, MDIO_NAME_LEN);
> mdiodev->read = smc911x_miiphy_read;
> mdiodev->write = smc911x_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 18/25] net: davinci_emac: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (16 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 17/25] net: smc911x: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:21 ` [PATCH 19/25] net: qe: uec: " Vladimir Oltean
` (6 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ti/davinci_emac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c
index bfe1b84cd566..2dfadbd82d5b 100644
--- a/drivers/net/ti/davinci_emac.c
+++ b/drivers/net/ti/davinci_emac.c
@@ -816,7 +816,7 @@ static int davinci_emac_probe(struct udevice *dev)
struct mii_dev *mdiodev = mdio_alloc();
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, phy[i].name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, phy[i].name, MDIO_NAME_LEN);
mdiodev->read = davinci_mii_phy_read;
mdiodev->write = davinci_mii_phy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 18/25] net: davinci_emac: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 18/25] net: davinci_emac: " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ti/davinci_emac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ti/davinci_emac.c b/drivers/net/ti/davinci_emac.c
> index bfe1b84cd566..2dfadbd82d5b 100644
> --- a/drivers/net/ti/davinci_emac.c
> +++ b/drivers/net/ti/davinci_emac.c
> @@ -816,7 +816,7 @@ static int davinci_emac_probe(struct udevice *dev)
> struct mii_dev *mdiodev = mdio_alloc();
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, phy[i].name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, phy[i].name, MDIO_NAME_LEN);
> mdiodev->read = davinci_mii_phy_read;
> mdiodev->write = davinci_mii_phy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 19/25] net: qe: uec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (17 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 18/25] net: davinci_emac: " Vladimir Oltean
@ 2021-09-27 11:21 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:22 ` [PATCH 20/25] net: mdio-uclass: rewrite dm_mdio_post_probe using strlcpy Vladimir Oltean
` (5 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:21 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/qe/uec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index 5da971ddc0af..c4bd5c4a147f 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -1407,7 +1407,7 @@ int uec_initialize(struct bd_info *bis, struct uec_inf *uec_info)
if (!mdiodev)
return -ENOMEM;
- strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
+ strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
mdiodev->read = uec_miiphy_read;
mdiodev->write = uec_miiphy_write;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 19/25] net: qe: uec: ensure mdiodev->name is NULL terminated after MDIO_NAME_LEN truncation
2021-09-27 11:21 ` [PATCH 19/25] net: qe: uec: " Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass MDIO_NAME_LEN - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/qe/uec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index 5da971ddc0af..c4bd5c4a147f 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -1407,7 +1407,7 @@ int uec_initialize(struct bd_info *bis, struct uec_inf *uec_info)
>
> if (!mdiodev)
> return -ENOMEM;
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strlcpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = uec_miiphy_read;
> mdiodev->write = uec_miiphy_write;
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 20/25] net: mdio-uclass: rewrite dm_mdio_post_probe using strlcpy
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (18 preceding siblings ...)
2021-09-27 11:21 ` [PATCH 19/25] net: qe: uec: " Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:22 ` [PATCH 21/25] scripts: ensure the cocci script for miiphy_register does not leave NULL-unterminated strings Vladimir Oltean
` (4 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
dm_mdio_post_probe used to be vulnerable after truncation, but has been
patched by commit 398e7512d8d7 ("net: Fix Covarity Defect 244093").
Nonetheless, we can use strlcpy like the rest of the code base now,
which yields the same result.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/mdio-uclass.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
index 1b687765b8ca..e74e34f78f9c 100644
--- a/net/mdio-uclass.c
+++ b/net/mdio-uclass.c
@@ -101,7 +101,7 @@ static int dm_mdio_post_probe(struct udevice *dev)
pdata->mii_bus->write = mdio_write;
pdata->mii_bus->reset = mdio_reset;
pdata->mii_bus->priv = dev;
- strncpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN - 1);
+ strlcpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN);
return mdio_register(pdata->mii_bus);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 20/25] net: mdio-uclass: rewrite dm_mdio_post_probe using strlcpy
2021-09-27 11:22 ` [PATCH 20/25] net: mdio-uclass: rewrite dm_mdio_post_probe using strlcpy Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> dm_mdio_post_probe used to be vulnerable after truncation, but has been
> patched by commit 398e7512d8d7 ("net: Fix Covarity Defect 244093").
> Nonetheless, we can use strlcpy like the rest of the code base now,
> which yields the same result.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> net/mdio-uclass.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> index 1b687765b8ca..e74e34f78f9c 100644
> --- a/net/mdio-uclass.c
> +++ b/net/mdio-uclass.c
> @@ -101,7 +101,7 @@ static int dm_mdio_post_probe(struct udevice *dev)
> pdata->mii_bus->write = mdio_write;
> pdata->mii_bus->reset = mdio_reset;
> pdata->mii_bus->priv = dev;
> - strncpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN - 1);
> + strlcpy(pdata->mii_bus->name, dev->name, MDIO_NAME_LEN);
>
> return mdio_register(pdata->mii_bus);
> }
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 21/25] scripts: ensure the cocci script for miiphy_register does not leave NULL-unterminated strings
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (19 preceding siblings ...)
2021-09-27 11:22 ` [PATCH 20/25] net: mdio-uclass: rewrite dm_mdio_post_probe using strlcpy Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
2021-09-27 11:22 ` [PATCH 22/25] net: dsa: felix: check return code of mdio_alloc and mdio_register Vladimir Oltean
` (3 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass MDIO_NAME_LEN - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
scripts/coccinelle/net/mdio_register.cocci | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/coccinelle/net/mdio_register.cocci b/scripts/coccinelle/net/mdio_register.cocci
index 100f10293610..31a40360f99e 100644
--- a/scripts/coccinelle/net/mdio_register.cocci
+++ b/scripts/coccinelle/net/mdio_register.cocci
@@ -16,7 +16,7 @@ identifier readfunc, writefunc;
- miiphy_register(devname, readfunc, writefunc);
+ struct mii_dev *mdiodev = mdio_alloc();
+ if (!mdiodev) return -ENOMEM;
-+ strncpy(mdiodev->name, devname, MDIO_NAME_LEN);
++ strlcpy(mdiodev->name, devname, MDIO_NAME_LEN);
+ mdiodev->read = readfunc;
+ mdiodev->write = writefunc;
+
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 22/25] net: dsa: felix: check return code of mdio_alloc and mdio_register
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (20 preceding siblings ...)
2021-09-27 11:22 ` [PATCH 21/25] scripts: ensure the cocci script for miiphy_register does not leave NULL-unterminated strings Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:22 ` [PATCH 23/25] net: dsa: ensure port names are NULL-terminated after DSA_PORT_NAME_LENGTH truncation Vladimir Oltean
` (2 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
These functions can return errors, it's best to catch them and trigger
the driver unwind code path.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/mscc_eswitch/felix_switch.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
index 4c2e57755967..98ae39e81d65 100644
--- a/drivers/net/mscc_eswitch/felix_switch.c
+++ b/drivers/net/mscc_eswitch/felix_switch.c
@@ -276,6 +276,7 @@ static void felix_init(struct udevice *dev)
static int felix_probe(struct udevice *dev)
{
struct felix_priv *priv = dev_get_priv(dev);
+ int err;
if (ofnode_valid(dev_ofnode(dev)) &&
!ofnode_is_available(dev_ofnode(dev))) {
@@ -300,11 +301,18 @@ static int felix_probe(struct udevice *dev)
struct mii_dev *mii_bus;
mii_bus = mdio_alloc();
+ if (!mii_bus)
+ return -ENOMEM;
+
mii_bus->read = felix_mdio_read;
mii_bus->write = felix_mdio_write;
mii_bus->priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
strlcpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
- mdio_register(mii_bus);
+ err = mdio_register(mii_bus);
+ if (err) {
+ mdio_free(mii_bus);
+ return err;
+ }
}
dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 22/25] net: dsa: felix: check return code of mdio_alloc and mdio_register
2021-09-27 11:22 ` [PATCH 22/25] net: dsa: felix: check return code of mdio_alloc and mdio_register Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> These functions can return errors, it's best to catch them and trigger
> the driver unwind code path.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/mscc_eswitch/felix_switch.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
> index 4c2e57755967..98ae39e81d65 100644
> --- a/drivers/net/mscc_eswitch/felix_switch.c
> +++ b/drivers/net/mscc_eswitch/felix_switch.c
> @@ -276,6 +276,7 @@ static void felix_init(struct udevice *dev)
> static int felix_probe(struct udevice *dev)
> {
> struct felix_priv *priv = dev_get_priv(dev);
> + int err;
>
> if (ofnode_valid(dev_ofnode(dev)) &&
> !ofnode_is_available(dev_ofnode(dev))) {
> @@ -300,11 +301,18 @@ static int felix_probe(struct udevice *dev)
> struct mii_dev *mii_bus;
>
> mii_bus = mdio_alloc();
> + if (!mii_bus)
> + return -ENOMEM;
> +
> mii_bus->read = felix_mdio_read;
> mii_bus->write = felix_mdio_write;
> mii_bus->priv = priv->imdio_base + FELIX_PM_IMDIO_BASE;
> strlcpy(mii_bus->name, dev->name, MDIO_NAME_LEN);
> - mdio_register(mii_bus);
> + err = mdio_register(mii_bus);
> + if (err) {
> + mdio_free(mii_bus);
> + return err;
> + }
> }
>
> dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 23/25] net: dsa: ensure port names are NULL-terminated after DSA_PORT_NAME_LENGTH truncation
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (21 preceding siblings ...)
2021-09-27 11:22 ` [PATCH 22/25] net: dsa: felix: check return code of mdio_alloc and mdio_register Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
2021-09-28 13:33 ` Ramon Fried
2021-09-27 11:22 ` [PATCH 24/25] arch: powerpc: mpc85xx: free MDIO bus if mdio_register fails Vladimir Oltean
2021-09-27 11:22 ` [PATCH 25/25] scripts: ensure the cocci script for miiphy_register does not leak the MDIO bus Vladimir Oltean
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
strncpy() simply bails out when copying a source string whose size
exceeds the destination string size, potentially leaving the destination
string unterminated.
One possible way to address is to pass DSA_PORT_NAME_LENGTH - 1 and a
previously zero-initialized destination string, but this is more
difficult to maintain.
The chosen alternative is to use strlcpy(), which properly limits the
copy len in the (srclen >= size) case to "size - 1", and which is also
more efficient than the strncpy() byte-by-byte implementation by using
memcpy. The destination string returned by strlcpy() is always NULL
terminated.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa-uclass.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 9b8ae1e82b92..8db0de686e2a 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -221,7 +221,7 @@ static int dsa_port_of_to_pdata(struct udevice *pdev)
label = ofnode_read_string(dev_ofnode(pdev), "label");
if (label)
- strncpy(port_pdata->name, label, DSA_PORT_NAME_LENGTH);
+ strlcpy(port_pdata->name, label, DSA_PORT_NAME_LENGTH);
eth_pdata = dev_get_plat(pdev);
eth_pdata->priv_pdata = port_pdata;
@@ -433,7 +433,7 @@ static int dsa_post_bind(struct udevice *dev)
struct dsa_port_pdata *port_pdata;
port_pdata = dev_get_parent_plat(pdev);
- strncpy(port_pdata->name, name, DSA_PORT_NAME_LENGTH);
+ strlcpy(port_pdata->name, name, DSA_PORT_NAME_LENGTH);
pdev->name = port_pdata->name;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 23/25] net: dsa: ensure port names are NULL-terminated after DSA_PORT_NAME_LENGTH truncation
2021-09-27 11:22 ` [PATCH 23/25] net: dsa: ensure port names are NULL-terminated after DSA_PORT_NAME_LENGTH truncation Vladimir Oltean
@ 2021-09-28 13:33 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:33 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> strncpy() simply bails out when copying a source string whose size
> exceeds the destination string size, potentially leaving the destination
> string unterminated.
>
> One possible way to address is to pass DSA_PORT_NAME_LENGTH - 1 and a
> previously zero-initialized destination string, but this is more
> difficult to maintain.
>
> The chosen alternative is to use strlcpy(), which properly limits the
> copy len in the (srclen >= size) case to "size - 1", and which is also
> more efficient than the strncpy() byte-by-byte implementation by using
> memcpy. The destination string returned by strlcpy() is always NULL
> terminated.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> net/dsa-uclass.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
> index 9b8ae1e82b92..8db0de686e2a 100644
> --- a/net/dsa-uclass.c
> +++ b/net/dsa-uclass.c
> @@ -221,7 +221,7 @@ static int dsa_port_of_to_pdata(struct udevice *pdev)
>
> label = ofnode_read_string(dev_ofnode(pdev), "label");
> if (label)
> - strncpy(port_pdata->name, label, DSA_PORT_NAME_LENGTH);
> + strlcpy(port_pdata->name, label, DSA_PORT_NAME_LENGTH);
>
> eth_pdata = dev_get_plat(pdev);
> eth_pdata->priv_pdata = port_pdata;
> @@ -433,7 +433,7 @@ static int dsa_post_bind(struct udevice *dev)
> struct dsa_port_pdata *port_pdata;
>
> port_pdata = dev_get_parent_plat(pdev);
> - strncpy(port_pdata->name, name, DSA_PORT_NAME_LENGTH);
> + strlcpy(port_pdata->name, name, DSA_PORT_NAME_LENGTH);
> pdev->name = port_pdata->name;
> }
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 24/25] arch: powerpc: mpc85xx: free MDIO bus if mdio_register fails
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (22 preceding siblings ...)
2021-09-27 11:22 ` [PATCH 23/25] net: dsa: ensure port names are NULL-terminated after DSA_PORT_NAME_LENGTH truncation Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
2021-09-28 13:27 ` Ramon Fried
2021-09-27 11:22 ` [PATCH 25/25] scripts: ensure the cocci script for miiphy_register does not leak the MDIO bus Vladimir Oltean
24 siblings, 1 reply; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
If mdio_register fails, it is nice to not leave behind dangling
allocated memory.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
arch/powerpc/cpu/mpc85xx/ether_fcc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
index 1f6f55707321..5cf0a3fb227a 100644
--- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c
+++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
@@ -449,8 +449,10 @@ int fec_initialize(struct bd_info *bis)
mdiodev->write = bb_miiphy_write;
retval = mdio_register(mdiodev);
- if (retval < 0)
+ if (retval < 0) {
+ mdio_free(mdiodev);
return retval;
+ }
#endif
}
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 24/25] arch: powerpc: mpc85xx: free MDIO bus if mdio_register fails
2021-09-27 11:22 ` [PATCH 24/25] arch: powerpc: mpc85xx: free MDIO bus if mdio_register fails Vladimir Oltean
@ 2021-09-28 13:27 ` Ramon Fried
0 siblings, 0 replies; 49+ messages in thread
From: Ramon Fried @ 2021-09-28 13:27 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: U-Boot Mailing List, Joe Hershberger
On Mon, Sep 27, 2021 at 2:22 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> If mdio_register fails, it is nice to not leave behind dangling
> allocated memory.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> arch/powerpc/cpu/mpc85xx/ether_fcc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> index 1f6f55707321..5cf0a3fb227a 100644
> --- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> +++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c
> @@ -449,8 +449,10 @@ int fec_initialize(struct bd_info *bis)
> mdiodev->write = bb_miiphy_write;
>
> retval = mdio_register(mdiodev);
> - if (retval < 0)
> + if (retval < 0) {
> + mdio_free(mdiodev);
> return retval;
> + }
> #endif
> }
>
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 25/25] scripts: ensure the cocci script for miiphy_register does not leak the MDIO bus
2021-09-27 11:21 [PATCH 00/25] Fix some non-NULL terminated strings in the networking subsystem Vladimir Oltean
` (23 preceding siblings ...)
2021-09-27 11:22 ` [PATCH 24/25] arch: powerpc: mpc85xx: free MDIO bus if mdio_register fails Vladimir Oltean
@ 2021-09-27 11:22 ` Vladimir Oltean
24 siblings, 0 replies; 49+ messages in thread
From: Vladimir Oltean @ 2021-09-27 11:22 UTC (permalink / raw)
To: u-boot; +Cc: Joe Hershberger, Ramon Fried
When mdio_register fails, mdio_free should be called on the mdiodev that
was previously allocated with mdio_alloc.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
scripts/coccinelle/net/mdio_register.cocci | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/coccinelle/net/mdio_register.cocci b/scripts/coccinelle/net/mdio_register.cocci
index 31a40360f99e..7d11281f4678 100644
--- a/scripts/coccinelle/net/mdio_register.cocci
+++ b/scripts/coccinelle/net/mdio_register.cocci
@@ -21,7 +21,7 @@ identifier readfunc, writefunc;
+ mdiodev->write = writefunc;
+
+ retval = mdio_register(mdiodev);
-+ if (retval < 0) return retval;
++ if (retval < 0) { mdio_free(mdiodev); return retval; }
@ update_read_sig @
identifier mii_reg.readfunc;
--
2.25.1
^ permalink raw reply related [flat|nested] 49+ messages in thread