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

[Joe Hershberger]

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.

Maybe ETH_ENETADDR_ENV_NAME_LEN?

>  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