diff mbox series

brcmfmac: Transform compatible string for FW loading

Message ID 20200625160725.31581-1-matthias.bgg@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series brcmfmac: Transform compatible string for FW loading | expand

Commit Message

Matthias Brugger June 25, 2020, 4:07 p.m. UTC 
From: Matthias Brugger <mbrugger@suse.com>

The driver relies on the compatible string from DT to determine which
FW configuration file it should load. The DTS spec allows for '/' as
part of the compatible string. We change this to '-' so that we will
still be able to load the config file, even when the compatible has a
'/'. This fixes explicitly the firmware loading for
"solidrun,cubox-i/q".

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/of.c  | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Hans de Goede June 25, 2020, 5:09 p.m. UTC | #1 
Hi,

On 6/25/20 6:07 PM, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> The driver relies on the compatible string from DT to determine which
> FW configuration file it should load. The DTS spec allows for '/' as
> part of the compatible string. We change this to '-' so that we will
> still be able to load the config file, even when the compatible has a
> '/'. This fixes explicitly the firmware loading for
> "solidrun,cubox-i/q".
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> ---
>   .../wireless/broadcom/brcm80211/brcmfmac/of.c  | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> index b886b56a5e5a..8a41b7f9cad3 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -17,7 +17,6 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   {
>   	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
>   	struct device_node *root, *np = dev->of_node;
> -	struct property *prop;
>   	int irq;
>   	u32 irqf;
>   	u32 val;
> @@ -25,8 +24,21 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   	/* Set board-type to the first string of the machine compatible prop */
>   	root = of_find_node_by_path("/");
>   	if (root) {
> -		prop = of_find_property(root, "compatible", NULL);
> -		settings->board_type = of_prop_next_string(prop, NULL);
> +		int i;
> +		char *board_type;
> +		const char *tmp;
> +
> +		of_property_read_string_index(root, "compatible", 0, &tmp);
> +
> +		/* get rid of '/' in the compatible string to be able to find the FW */
> +		board_type = devm_kzalloc(dev, strlen(tmp), GFP_KERNEL);

strlen() needs to be strlen() + 1 here to make place for the terminating zero.

> +		strncpy(board_type, tmp, strlen(tmp));

Please do not us strncpy, it is THE worst strcpy function
in existence, it does not guarantee 0 termination, so
it sucks, it sucks a lot do not use, thanks.

Instead use strlcpy or snprintf(..., "%s", ...)

> +		for (i = 0; i < strlen(board_type); i++) {

(The strlen here relies on there being a 0 behind the memory you
allocated because of the missing + 1)

> +			if (board_type[i] == '/')
> +				board_type[i] = '-';
> +		}
> +		settings->board_type = board_type;
> +
>   		of_node_put(root);
>   	}
>   
> 

Otherwise this looks good to me.

Regards,

Hans
kernel test robot June 25, 2020, 8:43 p.m. UTC | #2 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master sparc-next/master v5.8-rc2 next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/matthias-bgg-kernel-org/brcmfmac-Transform-compatible-string-for-FW-loading/20200626-001324
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c: In function 'brcmf_of_probe':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c:35:3: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
      35 |   strncpy(board_type, tmp, strlen(tmp));
         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/strncpy +35 drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c

    14	
    15	void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
    16			    struct brcmf_mp_device *settings)
    17	{
    18		struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
    19		struct device_node *root, *np = dev->of_node;
    20		int irq;
    21		u32 irqf;
    22		u32 val;
    23	
    24		/* Set board-type to the first string of the machine compatible prop */
    25		root = of_find_node_by_path("/");
    26		if (root) {
    27			int i;
    28			char *board_type;
    29			const char *tmp;
    30	
    31			of_property_read_string_index(root, "compatible", 0, &tmp);
    32	
    33			/* get rid of '/' in the compatible string to be able to find the FW */
    34			board_type = devm_kzalloc(dev, strlen(tmp), GFP_KERNEL);
  > 35			strncpy(board_type, tmp, strlen(tmp));

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot June 25, 2020, 9 p.m. UTC | #3 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master sparc-next/master v5.8-rc2 next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/matthias-bgg-kernel-org/brcmfmac-Transform-compatible-string-for-FW-loading/20200626-001324
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/bitmap.h:9,
                    from include/linux/cpumask.h:12,
                    from arch/mips/include/asm/processor.h:15,
                    from arch/mips/include/asm/thread_info.h:16,
                    from include/linux/thread_info.h:38,
                    from include/asm-generic/current.h:5,
                    from ./arch/mips/include/generated/asm/current.h:1,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:12,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c:6:
   In function 'strncpy',
       inlined from 'brcmf_of_probe' at drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c:35:3:
>> include/linux/string.h:297:30: warning: '__builtin_strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
     297 | #define __underlying_strncpy __builtin_strncpy
         |                              ^
   include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
     307 |  return __underlying_strncpy(p, q, size);
         |         ^~~~~~~~~~~~~~~~~~~~
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c: In function 'brcmf_of_probe':
   include/linux/string.h:295:29: note: length computed here
     295 | #define __underlying_strlen __builtin_strlen
         |                             ^
>> include/linux/string.h:328:10: note: in expansion of macro '__underlying_strlen'
     328 |   return __underlying_strlen(p);
         |          ^~~~~~~~~~~~~~~~~~~

vim +/__builtin_strncpy +297 include/linux/string.h

47227d27e2fcb01 Daniel Axtens 2020-06-03  275  
47227d27e2fcb01 Daniel Axtens 2020-06-03  276  #ifdef CONFIG_KASAN
47227d27e2fcb01 Daniel Axtens 2020-06-03  277  extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr);
47227d27e2fcb01 Daniel Axtens 2020-06-03  278  extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp);
47227d27e2fcb01 Daniel Axtens 2020-06-03  279  extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy);
47227d27e2fcb01 Daniel Axtens 2020-06-03  280  extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove);
47227d27e2fcb01 Daniel Axtens 2020-06-03  281  extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset);
47227d27e2fcb01 Daniel Axtens 2020-06-03  282  extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat);
47227d27e2fcb01 Daniel Axtens 2020-06-03  283  extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy);
47227d27e2fcb01 Daniel Axtens 2020-06-03  284  extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen);
47227d27e2fcb01 Daniel Axtens 2020-06-03  285  extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat);
47227d27e2fcb01 Daniel Axtens 2020-06-03  286  extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy);
47227d27e2fcb01 Daniel Axtens 2020-06-03  287  #else
47227d27e2fcb01 Daniel Axtens 2020-06-03  288  #define __underlying_memchr	__builtin_memchr
47227d27e2fcb01 Daniel Axtens 2020-06-03  289  #define __underlying_memcmp	__builtin_memcmp
47227d27e2fcb01 Daniel Axtens 2020-06-03  290  #define __underlying_memcpy	__builtin_memcpy
47227d27e2fcb01 Daniel Axtens 2020-06-03  291  #define __underlying_memmove	__builtin_memmove
47227d27e2fcb01 Daniel Axtens 2020-06-03  292  #define __underlying_memset	__builtin_memset
47227d27e2fcb01 Daniel Axtens 2020-06-03  293  #define __underlying_strcat	__builtin_strcat
47227d27e2fcb01 Daniel Axtens 2020-06-03  294  #define __underlying_strcpy	__builtin_strcpy
47227d27e2fcb01 Daniel Axtens 2020-06-03  295  #define __underlying_strlen	__builtin_strlen
47227d27e2fcb01 Daniel Axtens 2020-06-03  296  #define __underlying_strncat	__builtin_strncat
47227d27e2fcb01 Daniel Axtens 2020-06-03 @297  #define __underlying_strncpy	__builtin_strncpy
47227d27e2fcb01 Daniel Axtens 2020-06-03  298  #endif
47227d27e2fcb01 Daniel Axtens 2020-06-03  299  
6974f0c4555e285 Daniel Micay  2017-07-12  300  __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
6974f0c4555e285 Daniel Micay  2017-07-12  301  {
6974f0c4555e285 Daniel Micay  2017-07-12  302  	size_t p_size = __builtin_object_size(p, 0);
6974f0c4555e285 Daniel Micay  2017-07-12  303  	if (__builtin_constant_p(size) && p_size < size)
6974f0c4555e285 Daniel Micay  2017-07-12  304  		__write_overflow();
6974f0c4555e285 Daniel Micay  2017-07-12  305  	if (p_size < size)
6974f0c4555e285 Daniel Micay  2017-07-12  306  		fortify_panic(__func__);
47227d27e2fcb01 Daniel Axtens 2020-06-03  307  	return __underlying_strncpy(p, q, size);
6974f0c4555e285 Daniel Micay  2017-07-12  308  }
6974f0c4555e285 Daniel Micay  2017-07-12  309  
6974f0c4555e285 Daniel Micay  2017-07-12  310  __FORTIFY_INLINE char *strcat(char *p, const char *q)
6974f0c4555e285 Daniel Micay  2017-07-12  311  {
6974f0c4555e285 Daniel Micay  2017-07-12  312  	size_t p_size = __builtin_object_size(p, 0);
6974f0c4555e285 Daniel Micay  2017-07-12  313  	if (p_size == (size_t)-1)
47227d27e2fcb01 Daniel Axtens 2020-06-03  314  		return __underlying_strcat(p, q);
6974f0c4555e285 Daniel Micay  2017-07-12  315  	if (strlcat(p, q, p_size) >= p_size)
6974f0c4555e285 Daniel Micay  2017-07-12  316  		fortify_panic(__func__);
6974f0c4555e285 Daniel Micay  2017-07-12  317  	return p;
6974f0c4555e285 Daniel Micay  2017-07-12  318  }
6974f0c4555e285 Daniel Micay  2017-07-12  319  
6974f0c4555e285 Daniel Micay  2017-07-12  320  __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
6974f0c4555e285 Daniel Micay  2017-07-12  321  {
6974f0c4555e285 Daniel Micay  2017-07-12  322  	__kernel_size_t ret;
6974f0c4555e285 Daniel Micay  2017-07-12  323  	size_t p_size = __builtin_object_size(p, 0);
146734b091430c8 Arnd Bergmann 2017-12-14  324  
146734b091430c8 Arnd Bergmann 2017-12-14  325  	/* Work around gcc excess stack consumption issue */
146734b091430c8 Arnd Bergmann 2017-12-14  326  	if (p_size == (size_t)-1 ||
146734b091430c8 Arnd Bergmann 2017-12-14  327  	    (__builtin_constant_p(p[p_size - 1]) && p[p_size - 1] == '\0'))
47227d27e2fcb01 Daniel Axtens 2020-06-03 @328  		return __underlying_strlen(p);
6974f0c4555e285 Daniel Micay  2017-07-12  329  	ret = strnlen(p, p_size);
6974f0c4555e285 Daniel Micay  2017-07-12  330  	if (p_size <= ret)
6974f0c4555e285 Daniel Micay  2017-07-12  331  		fortify_panic(__func__);
6974f0c4555e285 Daniel Micay  2017-07-12  332  	return ret;
6974f0c4555e285 Daniel Micay  2017-07-12  333  }
6974f0c4555e285 Daniel Micay  2017-07-12  334  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index b886b56a5e5a..8a41b7f9cad3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -17,7 +17,6 @@  void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 {
 	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
 	struct device_node *root, *np = dev->of_node;
-	struct property *prop;
 	int irq;
 	u32 irqf;
 	u32 val;
@@ -25,8 +24,21 @@  void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 	/* Set board-type to the first string of the machine compatible prop */
 	root = of_find_node_by_path("/");
 	if (root) {
-		prop = of_find_property(root, "compatible", NULL);
-		settings->board_type = of_prop_next_string(prop, NULL);
+		int i;
+		char *board_type;
+		const char *tmp;
+
+		of_property_read_string_index(root, "compatible", 0, &tmp);
+
+		/* get rid of '/' in the compatible string to be able to find the FW */
+		board_type = devm_kzalloc(dev, strlen(tmp), GFP_KERNEL);
+		strncpy(board_type, tmp, strlen(tmp));
+		for (i = 0; i < strlen(board_type); i++) {
+			if (board_type[i] == '/')
+				board_type[i] = '-';
+		}
+		settings->board_type = board_type;
+
 		of_node_put(root);
 	}