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 |
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
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
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 --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); }