[U-Boot] [PATCH 0/2] Ethernet name length changes
The ethernet name should be within the ETH_NAME_LEN, as this is the buffer space allocated to ethernet name.
Otherwise, this causes buffer overflow.
At the same time the 16 char ethernet name size is inadequate to hold the name of ethernet "DPMAC17@rgmii-id", which is a valid name in upcoming LX2160AQDS/LX2160ARDB boards.
These patches try to solve the above two issues: 1. first patch honors the limit imposed by ETH_NAME_LEN on ethernet name 2. second patch increases this limit to accommodate the requirements of LX2160AQDS/LX2160ARDB
Cc: Varun Sethi V.Sethi@nxp.com
Pankaj Bansal (2): fsl/mc: Limit the ethernet name to ETH_NAME_LEN net: Increase ethernet name string size to 20 chars
drivers/net/fsl-mc/mc.c | 6 +++--- drivers/net/ldpaa_eth/ldpaa_eth.c | 4 ++-- include/net.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
The ethernet name should be within the ETH_NAME_LEN, as this is the buffer space allocated to ethernet name.
Otherwise, this causes buffer overflow.
Reported-by: Ioana Ciornei ioana.ciornei@nxp.com Signed-off-by: Pankaj Bansal pankaj.bansal@nxp.com --- drivers/net/fsl-mc/mc.c | 6 +++--- drivers/net/ldpaa_eth/ldpaa_eth.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index e2341a2e3c..33e4bf23e9 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -322,7 +322,7 @@ static int mc_fixup_dpc_mac_addr(void *blob, int dpmac_id, static int mc_fixup_mac_addrs(void *blob, enum mc_fixup_type type) { int i, err = 0, ret = 0; - char ethname[10]; + char ethname[ETH_NAME_LEN]; struct eth_device *eth_dev;
for (i = WRIOP1_DPMAC1; i < NUM_WRIOP_PORTS; i++) { @@ -330,8 +330,8 @@ static int mc_fixup_mac_addrs(void *blob, enum mc_fixup_type type) if (wriop_is_enabled_dpmac(i) != 1) continue;
- sprintf(ethname, "DPMAC%d@%s", i, - phy_interface_strings[wriop_get_enet_if(i)]); + snprintf(ethname, ETH_NAME_LEN, "DPMAC%d@%s", i, + phy_interface_strings[wriop_get_enet_if(i)]);
eth_dev = eth_get_dev_by_name(ethname); if (eth_dev == NULL) diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c index 4fc5a0626b..fe1c03e9e4 100644 --- a/drivers/net/ldpaa_eth/ldpaa_eth.c +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c @@ -1028,8 +1028,8 @@ static int ldpaa_eth_netdev_init(struct eth_device *net_dev, int err; struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)net_dev->priv;
- sprintf(net_dev->name, "DPMAC%d@%s", priv->dpmac_id, - phy_interface_strings[enet_if]); + snprintf(net_dev->name, ETH_NAME_LEN, "DPMAC%d@%s", priv->dpmac_id, + phy_interface_strings[enet_if]);
net_dev->iobase = 0; net_dev->init = ldpaa_eth_open;
On 08/01/2018 10:31 PM, Pankaj Bansal wrote:
The ethernet name should be within the ETH_NAME_LEN, as this is the buffer space allocated to ethernet name.
Otherwise, this causes buffer overflow.
Reported-by: Ioana Ciornei ioana.ciornei@nxp.com Signed-off-by: Pankaj Bansal pankaj.bansal@nxp.com
Applied to fsl-qoriq master, awaiting upstream. Thanks.
York
The 16 char ethernet name size is inadequate to hold the name of ethernet name "DPMAC17@rgmii-id", which is a valid name in LX2160AQDS/LX2160ARDB.
Therefore, increase the name string size to 20 chars.
Reported-by: Ioana Ciornei ioana.ciornei@nxp.com Suggested-by: Ioana Ciocoi Radulescu ruxandra.radulescu@nxp.com Signed-off-by: Pankaj Bansal pankaj.bansal@nxp.com --- include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 62f82c4dca..2b2deb5aae 100644 --- a/include/net.h +++ b/include/net.h @@ -164,7 +164,7 @@ void eth_halt_state_only(void); /* Set passive state */
#ifndef CONFIG_DM_ETH struct eth_device { -#define ETH_NAME_LEN 16 +#define ETH_NAME_LEN 20 char name[ETH_NAME_LEN]; unsigned char enetaddr[ARP_HLEN]; phys_addr_t iobase;
On 08/01/2018 10:31 PM, Pankaj Bansal wrote:
The 16 char ethernet name size is inadequate to hold the name of ethernet name "DPMAC17@rgmii-id", which is a valid name in LX2160AQDS/LX2160ARDB.
Therefore, increase the name string size to 20 chars.
Reported-by: Ioana Ciornei ioana.ciornei@nxp.com Suggested-by: Ioana Ciocoi Radulescu ruxandra.radulescu@nxp.com Signed-off-by: Pankaj Bansal pankaj.bansal@nxp.com
Applied to fsl-qoriq master, awaiting upstream. Thanks.
York
On Thu, Aug 2, 2018 at 6:01 AM, Pankaj Bansal pankaj.bansal@nxp.com wrote:
The ethernet name should be within the ETH_NAME_LEN, as this is the buffer space allocated to ethernet name.
Otherwise, this causes buffer overflow.
At the same time the 16 char ethernet name size is inadequate to hold the name of ethernet "DPMAC17@rgmii-id", which is a valid name in upcoming LX2160AQDS/LX2160ARDB boards.
Can you just shorten the name? This is a shared constant with Linux, so while we could change it, I'd prefer not to.
These patches try to solve the above two issues:
- first patch honors the limit imposed by ETH_NAME_LEN on ethernet name
- second patch increases this limit to accommodate the requirements of LX2160AQDS/LX2160ARDB
Cc: Varun Sethi V.Sethi@nxp.com
Pankaj Bansal (2): fsl/mc: Limit the ethernet name to ETH_NAME_LEN net: Increase ethernet name string size to 20 chars
drivers/net/fsl-mc/mc.c | 6 +++--- drivers/net/ldpaa_eth/ldpaa_eth.c | 4 ++-- include/net.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
-- 2.17.1 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
-----Original Message----- From: Joe Hershberger [mailto:joe.hershberger@ni.com] Sent: Thursday, August 2, 2018 10:15 PM To: Pankaj Bansal pankaj.bansal@nxp.com Cc: u-boot u-boot@lists.denx.de; Joe Hershberger joe.hershberger@ni.com; Varun Sethi V.Sethi@nxp.com Subject: Re: [U-Boot] [PATCH 0/2] Ethernet name length changes
On Thu, Aug 2, 2018 at 6:01 AM, Pankaj Bansal pankaj.bansal@nxp.com wrote:
The ethernet name should be within the ETH_NAME_LEN, as this is the buffer space allocated to ethernet name.
Otherwise, this causes buffer overflow.
At the same time the 16 char ethernet name size is inadequate to hold the name of ethernet "DPMAC17@rgmii-id", which is a valid name in upcoming LX2160AQDS/LX2160ARDB boards.
Can you just shorten the name? This is a shared constant with Linux, so while we could change it, I'd prefer not to.
Ok. Then you can just accept the first patch and leave the second one. With first patch we limit the ethernet name to ETH_NAME_LEN.
These patches try to solve the above two issues:
- first patch honors the limit imposed by ETH_NAME_LEN on ethernet
name 2. second patch increases this limit to accommodate the
requirements of
LX2160AQDS/LX2160ARDB
Cc: Varun Sethi V.Sethi@nxp.com
Pankaj Bansal (2): fsl/mc: Limit the ethernet name to ETH_NAME_LEN net: Increase ethernet name string size to 20 chars
drivers/net/fsl-mc/mc.c | 6 +++--- drivers/net/ldpaa_eth/ldpaa_eth.c | 4 ++-- include/net.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
-- 2.17.1 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
ts.denx.de%2Flistinfo%2Fu-
boot&data=02%7C01%7Cpankaj.bansal%40nxp.
com%7C11b8c79a3833492ed17608d5f8976b50%7C686ea1d3bc2b4c6fa9 2cd99c5c301
635%7C0%7C0%7C636688251592976394&sdata=gyGRsE%2B8MSXA U7IiycV7CtWGc
yabeUuBiTf9HJbVPQU%3D&reserved=0
participants (3)
-
Joe Hershberger -
Pankaj Bansal -
York Sun