[U-Boot] [PATCH 04/14] net: core: Sanitize get/set operations for enetaddr

[Olliver Schinagl]

Hey Joe,


On 30-11-16 20:20, Joe Hershberger wrote:
> On Fri, Nov 25, 2016 at 9:30 AM, Olliver Schinagl <oliver at schinagl.nl> wrote:
>> In the current net stack, we have a few functions to get and set
>> the "ethaddr" and "ethNaddr" environment variables, which use magic
>> values to get and set these environment variables. Remove the magicness
>> of the buffer by defining it proper and also check the input for its
>> length.
>>
>> Additionally use the define in fdt parser where the ethaddr variables
>> are also used.
>>
>> Signed-off-by: Olliver Schinagl <oliver at schinagl.nl>
>> ---
>>   common/fdt_support.c |  2 +-
>>   include/net.h        |  1 +
>>   net/eth_common.c     | 12 ++++++++----
>>   3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index b082662..89e6e47 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -469,7 +469,7 @@ void fdt_fixup_ethernet(void *fdt)
>>   {
>>          int i, j, prop;
>>          char *tmp, *end;
>> -       char mac[16];
>> +       char mac[ETH_ENETADDR_LEN];
>>          const char *path;
>>          unsigned char mac_addr[ARP_HLEN];
>>          int offset;
>> diff --git a/include/net.h b/include/net.h
>> index 9cd7870..2534913 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -243,6 +243,7 @@ void eth_set_current(void);         /* set nterface to ethcur var */
>>
>>   int eth_get_dev_index(void);           /* get the device index */
>>   void eth_parse_enetaddr(const char *addr, uchar *enetaddr);
>> +#define ETH_ENETADDR_LEN 32
> I'd prefer a clearer name here.
I'd agree, but
>
> Maybe ETH_ENETADDR_ENV_NAME_LEN?
I do find it to be quite a long name. Which makes the whole 80 charcters 
per line a thing.

But done :)
>
>>   int eth_getenv_enetaddr(const char *name, uchar *enetaddr);
>>   int eth_setenv_enetaddr(const char *name, const uchar *enetaddr);
>>
>> diff --git a/net/eth_common.c b/net/eth_common.c
>> index e9d3c66..079be89 100644
>> --- a/net/eth_common.c
>> +++ b/net/eth_common.c
>> @@ -42,16 +42,20 @@ int eth_setenv_enetaddr(const char *name, const uchar *enetaddr)
>>   int eth_getenv_enetaddr_by_index(const char *base_name, int index,
>>                                   uchar *enetaddr)
>>   {
>> -       char enetvar[32];
>> -       sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
>> +       char enetvar[ETH_ENETADDR_LEN];
>> +
>> +       snprintf(enetvar, ETH_ENETADDR_LEN, index ? "%s%daddr" : "%saddr",
>> +                base_name, index);
>>          return eth_getenv_enetaddr(enetvar, enetaddr);
>>   }
>>
>>   int eth_setenv_enetaddr_by_index(const char *base_name, int index,
>>                                   uchar *enetaddr)
>>   {
>> -       char enetvar[32];
>> -       sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
>> +       char enetvar[ETH_ENETADDR_LEN];
>> +
>> +       snprintf(enetvar, ETH_ENETADDR_LEN, index ? "%s%daddr" : "%saddr",
>> +                base_name, index);
>>          return eth_setenv_enetaddr(enetvar, enetaddr);
>>   }
>>
>> --
>> 2.10.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot