[U-Boot] Running out of memory after setting invalid ethaddr

[Thomas Schaefer]

Hi Joe et. al.

After setting MAC address to invalid value 00:00:00:00:00:00 and resetting to a valid one, u-boot will run into a recursive endless loop until no memory is left to allocate a new MAC address:

=> print eth1addr
eth1addr=00:a0:a5:10:20:31
=> setenv eth1addr 00:00:00:00:00:00
=> setenv eth1addr 00:a0:a5:10:20:31
## Can't malloc 18 bytes
=>

I have tested this with current v2017.11-rc4 on a Freescale T1024RDB eval system, but also see this on our custom i.MX7 and T1042 boards which are based on v2017.03.

After some investigation, I found that the recursive loop is entered in eth_write_hwaddr in net/eth_legacy.c when 'eth_env_set_enetaddr_by_index' is called:

~snip
        } else if (is_valid_ethaddr(dev->enetaddr)) {
                eth_env_set_enetaddr_by_index(base_name, eth_number,
                                              dev->enetaddr);
~snip

The patch shown below would prevent u-boot from setting ethaddr to an invalid value like 00:00:00:00:00:00 and avoids running into a situation as shown above:

diff --git a/net/eth_legacy.c b/net/eth_legacy.c
index be0cf64a3d..8d0a573551 100644
--- a/net/eth_legacy.c
+++ b/net/eth_legacy.c
@@ -104,9 +104,11 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
 {
        int index;
        struct eth_device *dev;
+       int ret = 0;
+       unsigned char tmp_enetaddr[ARP_HLEN];

        if (!eth_devices)
-               return 0;
+               return ret;

        /* look for an index after "eth" */
        index = simple_strtoul(name + 3, NULL, 10);
@@ -117,8 +119,17 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
                        switch (op) {
                        case env_op_create:
                        case env_op_overwrite:
-                               eth_parse_enetaddr(value, dev->enetaddr);
-                               eth_write_hwaddr(dev, "eth", dev->index);
+                               if (is_valid_ethaddr(tmp_enetaddr)) {
+                                       eth_parse_enetaddr(value,
+                                                          dev->enetaddr);
+                                       eth_write_hwaddr(dev, "eth",
+                                                        dev->index);
+                               } else {
+                                       printf("\nMAC address %pM is not "
+                                              "valid\n",
+                                              tmp_enetaddr);
+                                       ret = 1;
+                               }
                                break;
                        case env_op_delete:
                                memset(dev->enetaddr, 0, ARP_HLEN);
@@ -127,7 +138,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op,
                dev = dev->next;
        } while (dev != eth_devices);

-       return 0;
+       return ret;
 }
 U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr);


What do you think?

Best regards,
Thomas


Thomas Schäfer
SW Design Engineer

Kontron - An S&T Company
Heinrich-Barth-Straße 1-1a | 66115 Saarbrücken | Germany
P: +49 681 95916 203
thomas.schaefer at kontron.com