Re: brcmsmac: use sprom from bcma

[Hauke Mehrtens <hauke@hauke-m.de>]

On 05/17/2012 09:13 PM, Dan Carpenter wrote:
> Hello Hauke, Arend,
> 
> The patch 898d3c3b2462: "brcmsmac: use sprom from bcma" from Apr 29, 
> 2012, leads to the following warning:
> drivers/net/wireless/brcm80211/brcmsmac/channel.c:645 
> brcms_c_country_valid()
> 	 error: buffer overflow 'ccode' 2 <= 2
> 
> -       if (ccode && brcms_c_country_valid(ccode))
> -               strncpy(wlc->pub->srom_ccode, ccode, BRCM_CNTRY_BUF_SZ - 1);
> +       if (sprom->alpha2 && brcms_c_country_valid(sprom->alpha2))
>                                                    ^^^^^^^^^^^^^
> This is a two character array.  It's not NULL terminated.
> 
> +               strncpy(wlc->pub->srom_ccode, sprom->alpha2, sizeof(sprom->alpha2));
> 
> But in brcms_c_country_valid() we check for the NULL terminator.
> 
>    637  static bool brcms_c_country_valid(const char *ccode)
>    638  {
>    639          /*
>    640           * only allow ascii alpha uppercase for the first 2
>    641           * chars.
>    642           */
>    643          if (!((0x80 & ccode[0]) == 0 && ccode[0] >= 0x41 && ccode[0] <= 0x5A &&
>    644                (0x80 & ccode[1]) == 0 && ccode[1] >= 0x41 && ccode[1] <= 0x5A &&
>    645                ccode[2] == '\0'))
>                       ^^^^^^^^^^^^^^^^
> Here.
> 
>    646                  return false;
> 
> My guess is that this works because -> leddc_on_time is mostly zero.
> 
> regards,
> dan carpenter
> 
Hi Dan,

your guess is probably right, but I do not know want is the best
solution to fix this. I set this to 2 byte as there are just two bytes
memory for this in the sprom. In the nvram of some SoC I also found a 3
letter code ccode=US2 and an other wrong two letter code ccode=Q2. What
is the way we should handle this?

1. just read the first 2 bytes and ignore the rest -> change
brcms_c_country_valid() and some SoC parsing code.

2. read the first 2 bytes and reject longer codes as completely invalid
(probably just found in nvram on SoCs) -> change brcms_c_country_valid()

3. read 4 (or more) bytes and let brcmsmac decide what is a valid code
-> change sprom struct and some more code

I would vote for number 2.

@Arend by the way how should the code EU or 0 be handled? It is used on
all my recent SoCs.

Hauke