The wilc_sdio_read_size() calls wilc_sdio_cmd52() but does not check the return value. This could lead to execution with potentially invalid data if wilc_sdio_cmd52() fails. A proper implementation can be found in wilc_sdio_read_reg().
Add error handling for wilc_sdio_cmd52(). If wilc_sdio_cmd52() fails, log an error message via dev_err().
Fixes: 0e1af73ddeb9 ("staging/wilc1000: use proper naming for global symbols") Cc: stable@vger.kernel.org # v4.15 Signed-off-by: Wentao Liang vulab@iscas.ac.cn --- drivers/net/wireless/microchip/wilc1000/sdio.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c index 5262c8846c13..e7a2bc9f9902 100644 --- a/drivers/net/wireless/microchip/wilc1000/sdio.c +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c @@ -771,6 +771,8 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) { u32 tmp; struct sdio_cmd52 cmd; + struct sdio_func *func = dev_to_sdio_func(wilc->dev); + int ret;
/** * Read DMA count in words @@ -780,12 +782,20 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) cmd.raw = 0; cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG; cmd.data = 0; - wilc_sdio_cmd52(wilc, &cmd); + ret = wilc_sdio_cmd52(wilc, &cmd); + if (ret) { + dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); + return ret; + } tmp = cmd.data;
cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1; cmd.data = 0; - wilc_sdio_cmd52(wilc, &cmd); + ret = wilc_sdio_cmd52(wilc, &cmd); + if (ret) { + dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); + return ret; + } tmp |= (cmd.data << 8);
*size = tmp;
On Fri, 2025-05-16 at 15:50 +0200, Markus Elfring wrote:
…
Add error handling for wilc_sdio_cmd52(). If wilc_sdio_cmd52() fails, log an error message via dev_err().
Please avoid duplicate exception handling code. Can another jump target become nicer for this purpose?
<form letter> (stolen from Greg)
Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them.
Hi Wentao,
kernel test robot noticed the following build errors:
[auto build test ERROR on wireless-next/main] [also build test ERROR on wireless/main linus/master v6.15-rc6 next-20250516] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Wentao-Liang/wifi-wilc1000-Ad... base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main patch link: https://lore.kernel.org/r/20250516083842.903-1-vulab%40iscas.ac.cn patch subject: [PATCH] wifi: wilc1000: Add error handling for wilc_sdio_cmd52() config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250518/202505180440.dyQDHWJJ-lkp@i...) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250518/202505180440.dyQDHWJJ-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202505180440.dyQDHWJJ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/device.h:15, from include/linux/mmc/sdio_func.h:11, from drivers/net/wireless/microchip/wilc1000/sdio.c:8: drivers/net/wireless/microchip/wilc1000/sdio.c: In function 'wilc_sdio_read_size':
drivers/net/wireless/microchip/wilc1000/sdio.c:792:32: error: 'struct sdio_func' has no member named 'devm'; did you mean 'dev'?
792 | dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); | ^~~~ include/linux/dev_printk.h:110:25: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ drivers/net/wireless/microchip/wilc1000/sdio.c:792:17: note: in expansion of macro 'dev_err' 792 | dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); | ^~~~~~~ drivers/net/wireless/microchip/wilc1000/sdio.c:801:32: error: 'struct sdio_func' has no member named 'devm'; did you mean 'dev'? 801 | dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); | ^~~~ include/linux/dev_printk.h:110:25: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ drivers/net/wireless/microchip/wilc1000/sdio.c:801:17: note: in expansion of macro 'dev_err' 801 | dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); | ^~~~~~~
vim +792 drivers/net/wireless/microchip/wilc1000/sdio.c
774 775 static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) 776 { 777 u32 tmp; 778 struct sdio_cmd52 cmd; 779 struct sdio_func *func = dev_to_sdio_func(wilc->dev); 780 int ret; 781 782 /** 783 * Read DMA count in words 784 **/ 785 cmd.read_write = 0; 786 cmd.function = 0; 787 cmd.raw = 0; 788 cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG; 789 cmd.data = 0; 790 ret = wilc_sdio_cmd52(wilc, &cmd); 791 if (ret) {
792 dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n");
793 return ret; 794 } 795 tmp = cmd.data; 796 797 cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1; 798 cmd.data = 0; 799 ret = wilc_sdio_cmd52(wilc, &cmd); 800 if (ret) { 801 dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n"); 802 return ret; 803 } 804 tmp |= (cmd.data << 8); 805 806 *size = tmp; 807 return 0; 808 } 809
Hi Wentao,
Thanks for your patch!
On Fri, 16 May 2025 at 10:39, Wentao Liang vulab@iscas.ac.cn wrote:
The wilc_sdio_read_size() calls wilc_sdio_cmd52() but does not check the return value. This could lead to execution with potentially invalid data if wilc_sdio_cmd52() fails. A proper implementation can be found in wilc_sdio_read_reg().
Add error handling for wilc_sdio_cmd52(). If wilc_sdio_cmd52() fails, log an error message via dev_err().
There is no need to print an error message, as wilc_sdio_cmd52() already does that. Same with the existing wilc_sdio_read_reg(), and with all of its callers, which leads to multiple error messages for a single failure.
Fixes: 0e1af73ddeb9 ("staging/wilc1000: use proper naming for global symbols")
This is not the commit you are looking for...
Cc: stable@vger.kernel.org # v4.15 Signed-off-by: Wentao Liang vulab@iscas.ac.cn
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c @@ -771,6 +771,8 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) { u32 tmp; struct sdio_cmd52 cmd;
struct sdio_func *func = dev_to_sdio_func(wilc->dev);int ret; /** * Read DMA count in words@@ -780,12 +782,20 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) cmd.raw = 0; cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG; cmd.data = 0;
wilc_sdio_cmd52(wilc, &cmd);
ret = wilc_sdio_cmd52(wilc, &cmd);if (ret) {dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n");
Looks like the AI wasn't trained properly. Please try to (at least) test-compile your patches.
return ret;} tmp = cmd.data; cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1; cmd.data = 0;
wilc_sdio_cmd52(wilc, &cmd);
ret = wilc_sdio_cmd52(wilc, &cmd);if (ret) {dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n");
Likewise.
return ret;} tmp |= (cmd.data << 8); *size = tmp;
Gr{oetje,eeting}s,
Geert
linux-stable-mirror@lists.linaro.org
-
Geert Uytterhoeven -
Johannes Berg -
kernel test robot -
Markus Elfring -
Wentao Liang