* [PATCH] iwlegacy: Sanity check for sta_id
@ 2025-09-06 9:42 Chen Yufeng
2025-09-07 4:41 ` kernel test robot
2025-09-08 8:59 ` Stanislaw Gruszka
0 siblings, 2 replies; 3+ messages in thread
From: Chen Yufeng @ 2025-09-06 9:42 UTC (permalink / raw)
To: stf_xl; +Cc: linux-wireless, Chen Yufeng
This patch is similar to 2da424b0773c("iwlwifi: Sanity check for sta_id").
`2da424b0773c` introduced a sanity check to prevent potential memory
corruption in function `iwl_sta_ucode_activate`.
In the iwlegacy driver, the function `il_sta_ucode_activate` shares
a similar logic with the `iwl_sta_ucode_activate` function in iwlwifi.
Initial observations suggest that the function may not adequately
validate the range of the `sta_id` parameter. If `sta_id` exceeds
the expected range, it could result in memory corruption or crash.
Although there is no confirmation of a similar vulnerability in the
iwlegacy driver, it is recommended to adopt a preventive approach
by adding range checks for `sta_id` in the `il_sta_ucode_activate`
function. For example:
```
if (sta_id >= IL_STATION_COUNT) {
IL_ERR(il, "invalid sta_id %u", sta_id);
return -EINVAL;
}
```
Adding such boundary checks can effectively mitigate potential
memory corruption issues.
Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>
---
drivers/net/wireless/intel/iwlegacy/common.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index b7bd3ec4cc50..a3bcf9d9ffa2 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -1735,10 +1735,13 @@ il_cancel_scan_deferred_work(struct il_priv *il)
EXPORT_SYMBOL(il_cancel_scan_deferred_work);
/* il->sta_lock must be held */
-static void
+static int
il_sta_ucode_activate(struct il_priv *il, u8 sta_id)
{
-
+ if (sta_id >= IL_STATION_COUNT) {
+ IL_ERR(il, "invalid sta_id %u", sta_id);
+ return -EINVAL;
+ }
if (!(il->stations[sta_id].used & IL_STA_DRIVER_ACTIVE))
IL_ERR("ACTIVATE a non DRIVER active station id %u addr %pM\n",
sta_id, il->stations[sta_id].sta.sta.addr);
@@ -1752,6 +1755,7 @@ il_sta_ucode_activate(struct il_priv *il, u8 sta_id)
D_ASSOC("Added STA id %u addr %pM to uCode\n", sta_id,
il->stations[sta_id].sta.sta.addr);
}
+ return 0;
}
static int
@@ -1774,8 +1778,7 @@ il_process_add_sta_resp(struct il_priv *il, struct il_addsta_cmd *addsta,
switch (pkt->u.add_sta.status) {
case ADD_STA_SUCCESS_MSK:
D_INFO("C_ADD_STA PASSED\n");
- il_sta_ucode_activate(il, sta_id);
- ret = 0;
+ ret = il_sta_ucode_activate(il, sta_id);
break;
case ADD_STA_NO_ROOM_IN_TBL:
IL_ERR("Adding station %d failed, no room in table.\n", sta_id);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iwlegacy: Sanity check for sta_id
2025-09-06 9:42 [PATCH] iwlegacy: Sanity check for sta_id Chen Yufeng
@ 2025-09-07 4:41 ` kernel test robot
2025-09-08 8:59 ` Stanislaw Gruszka
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2025-09-07 4:41 UTC (permalink / raw)
To: Chen Yufeng, stf_xl; +Cc: llvm, oe-kbuild-all, linux-wireless, Chen Yufeng
Hi Chen,
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.17-rc4 next-20250905]
[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/Chen-Yufeng/iwlegacy-Sanity-check-for-sta_id/20250906-174354
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/r/20250906094232.1580-1-chenyufeng%40iie.ac.cn
patch subject: [PATCH] iwlegacy: Sanity check for sta_id
config: x86_64-buildonly-randconfig-002-20250907 (https://download.01.org/0day-ci/archive/20250907/202509071251.YuF4EGpk-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250907/202509071251.YuF4EGpk-lkp@intel.com/reproduce)
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/202509071251.YuF4EGpk-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/wireless/intel/iwlegacy/common.c:1742:3: error: incompatible pointer types initializing 'const char *' with an expression of type 'struct il_priv *' [-Werror,-Wincompatible-pointer-types]
1742 | IL_ERR(il, "invalid sta_id %u", sta_id);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/intel/iwlegacy/common.h:31:25: note: expanded from macro 'IL_ERR'
31 | #define IL_ERR(f, a...) dev_err(&il->pci_dev->dev, f, ## a)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:154:2: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
109 | dev_printk_index_emit(level, fmt); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
105 | printk_index_subsys_emit("%s %s: ", level, fmt)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:481:2: note: expanded from macro 'printk_index_subsys_emit'
481 | __printk_index_emit(fmt, level, subsys_fmt_prefix)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:447:12: note: expanded from macro '__printk_index_emit'
447 | .fmt = __builtin_constant_p(_fmt) ? (_fmt) : NULL, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/intel/iwlegacy/common.c:1742:10: error: incompatible pointer types passing 'struct il_priv *' to parameter of type 'const char *' [-Werror,-Wincompatible-pointer-types]
1742 | IL_ERR(il, "invalid sta_id %u", sta_id);
| ^~
drivers/net/wireless/intel/iwlegacy/common.h:31:52: note: expanded from macro 'IL_ERR'
31 | #define IL_ERR(f, a...) dev_err(&il->pci_dev->dev, f, ## a)
| ^
include/linux/dev_printk.h:154:57: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~
include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:50:53: note: passing argument to parameter 'fmt' here
50 | void _dev_err(const struct device *dev, const char *fmt, ...);
| ^
2 errors generated.
vim +1742 drivers/net/wireless/intel/iwlegacy/common.c
1736
1737 /* il->sta_lock must be held */
1738 static int
1739 il_sta_ucode_activate(struct il_priv *il, u8 sta_id)
1740 {
1741 if (sta_id >= IL_STATION_COUNT) {
> 1742 IL_ERR(il, "invalid sta_id %u", sta_id);
1743 return -EINVAL;
1744 }
1745 if (!(il->stations[sta_id].used & IL_STA_DRIVER_ACTIVE))
1746 IL_ERR("ACTIVATE a non DRIVER active station id %u addr %pM\n",
1747 sta_id, il->stations[sta_id].sta.sta.addr);
1748
1749 if (il->stations[sta_id].used & IL_STA_UCODE_ACTIVE) {
1750 D_ASSOC("STA id %u addr %pM already present"
1751 " in uCode (according to driver)\n", sta_id,
1752 il->stations[sta_id].sta.sta.addr);
1753 } else {
1754 il->stations[sta_id].used |= IL_STA_UCODE_ACTIVE;
1755 D_ASSOC("Added STA id %u addr %pM to uCode\n", sta_id,
1756 il->stations[sta_id].sta.sta.addr);
1757 }
1758 return 0;
1759 }
1760
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iwlegacy: Sanity check for sta_id
2025-09-06 9:42 [PATCH] iwlegacy: Sanity check for sta_id Chen Yufeng
2025-09-07 4:41 ` kernel test robot
@ 2025-09-08 8:59 ` Stanislaw Gruszka
1 sibling, 0 replies; 3+ messages in thread
From: Stanislaw Gruszka @ 2025-09-08 8:59 UTC (permalink / raw)
To: Chen Yufeng; +Cc: linux-wireless
Hi,
On Sat, Sep 06, 2025 at 05:42:32PM +0800, Chen Yufeng wrote:
> This patch is similar to 2da424b0773c("iwlwifi: Sanity check for sta_id").
> `2da424b0773c` introduced a sanity check to prevent potential memory
> corruption in function `iwl_sta_ucode_activate`.
>
> In the iwlegacy driver, the function `il_sta_ucode_activate` shares
> a similar logic with the `iwl_sta_ucode_activate` function in iwlwifi.
> Initial observations suggest that the function may not adequately
> validate the range of the `sta_id` parameter. If `sta_id` exceeds
> the expected range, it could result in memory corruption or crash.
>
> Although there is no confirmation of a similar vulnerability in the
> iwlegacy driver, it is recommended to adopt a preventive approach
> by adding range checks for `sta_id` in the `il_sta_ucode_activate`
> function. For example:
> ```
> if (sta_id >= IL_STATION_COUNT) {
> IL_ERR(il, "invalid sta_id %u", sta_id);
> return -EINVAL;
> }
> ```
> Adding such boundary checks can effectively mitigate potential
> memory corruption issues.
Ask your LLM to write a simple changelog instead of marketing fluff.
Something like: 'Add sanity check for il->stations[] array index.'.
It would be sufficient.
> Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>
> ---
> drivers/net/wireless/intel/iwlegacy/common.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
> index b7bd3ec4cc50..a3bcf9d9ffa2 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
> @@ -1735,10 +1735,13 @@ il_cancel_scan_deferred_work(struct il_priv *il)
> EXPORT_SYMBOL(il_cancel_scan_deferred_work);
>
> /* il->sta_lock must be held */
> -static void
> +static int
> il_sta_ucode_activate(struct il_priv *il, u8 sta_id)
> {
> -
> + if (sta_id >= IL_STATION_COUNT) {
> + IL_ERR(il, "invalid sta_id %u", sta_id);
Please compile check your changes.
> + return -EINVAL;
> + }
> if (!(il->stations[sta_id].used & IL_STA_DRIVER_ACTIVE))
> IL_ERR("ACTIVATE a non DRIVER active station id %u addr %pM\n",
> sta_id, il->stations[sta_id].sta.sta.addr);
> @@ -1752,6 +1755,7 @@ il_sta_ucode_activate(struct il_priv *il, u8 sta_id)
> D_ASSOC("Added STA id %u addr %pM to uCode\n", sta_id,
> il->stations[sta_id].sta.sta.addr);
> }
> + return 0;
> }
>
> static int
> @@ -1774,8 +1778,7 @@ il_process_add_sta_resp(struct il_priv *il, struct il_addsta_cmd *addsta,
This check should be done here, in il_process_add_sta_resp() since we
dereference il->stations[sta_id] in other places in this function.
Regards
Stanislaw
> switch (pkt->u.add_sta.status) {
> case ADD_STA_SUCCESS_MSK:
> D_INFO("C_ADD_STA PASSED\n");
> - il_sta_ucode_activate(il, sta_id);
> - ret = 0;
> + ret = il_sta_ucode_activate(il, sta_id);
> break;
> case ADD_STA_NO_ROOM_IN_TBL:
> IL_ERR("Adding station %d failed, no room in table.\n", sta_id);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-08 8:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-06 9:42 [PATCH] iwlegacy: Sanity check for sta_id Chen Yufeng
2025-09-07 4:41 ` kernel test robot
2025-09-08 8:59 ` Stanislaw Gruszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox