[PATCH v2] iwlegacy: Add sanity check for il->stations[] array index

[PATCH v2] iwlegacy: Add sanity check for il->stations[] array index

[Chen Yufeng]

In the il_process_add_sta_resp function, the index sta_id in 
il->stations[sta_id] is not validated, which may lead to memory 
corruption if the sta_id index is out of bounds.

Fixes: 0cdc21363cc2 ("iwlegacy: merge common .c files")

Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>
Reviewed-by: Stanislaw Gruszka <stf_xl@wp.pl>
---
Changes in v2:
- Add header file commands.h to get IL_STATION_COUNT
- move the validation of sta_id to il_process_add_sta_resp

 drivers/net/wireless/intel/iwlegacy/common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index b7bd3ec4cc50..2840d0935650 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -22,6 +22,7 @@
 #include <net/mac80211.h>
 
 #include "common.h"
+#include "commands.h"
 
 int
 _il_poll_bit(struct il_priv *il, u32 addr, u32 bits, u32 mask, int timeout)
@@ -1766,6 +1767,11 @@ il_process_add_sta_resp(struct il_priv *il, struct il_addsta_cmd *addsta,
 		IL_ERR("Bad return from C_ADD_STA (0x%08X)\n", pkt->hdr.flags);
 		return ret;
 	}
+	
+	if (sta_id >= IL_STATION_COUNT) {
+		IL_ERR(il, "invalid sta_id %u", sta_id);
+		return -EINVAL;
+	}
 
 	D_INFO("Processing response for adding station %u\n", sta_id);
 
-- 
2.34.1

Re: [PATCH v2] iwlegacy: Add sanity check for il->stations[] array index

[Johannes Berg]

On Thu, 2025-09-11 at 21:39 +0800, Chen Yufeng wrote:
> 
> Reviewed-by: Stanislaw Gruszka <stf_xl@wp.pl>

I don't know why now suddenly people have been adding Reviewed-by tags
for *others* ... do NOT do that!!

You are of course allowed/expected to add tags that were offered to you
by others in email, but I see no email from Stanislaw doing so.

johannes

Re: [PATCH v2] iwlegacy: Add sanity check for il->stations[] array index

[Stanislaw Gruszka]

On Thu, Sep 11, 2025 at 09:39:50PM +0800, Chen Yufeng wrote:
> In the il_process_add_sta_resp function, the index sta_id in 
> il->stations[sta_id] is not validated, which may lead to memory 
> corruption if the sta_id index is out of bounds.
> 
> Fixes: 0cdc21363cc2 ("iwlegacy: merge common .c files")
This tag is most likely not correct one. I don't think we need
one though.

> Signed-off-by: Chen Yufeng <chenyufeng@iie.ac.cn>
> Reviewed-by: Stanislaw Gruszka <stf_xl@wp.pl>
No, I haven't review the patch.

> ---
> Changes in v2:
> - Add header file commands.h to get IL_STATION_COUNT
> - move the validation of sta_id to il_process_add_sta_resp
> 
>  drivers/net/wireless/intel/iwlegacy/common.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
> index b7bd3ec4cc50..2840d0935650 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
> @@ -22,6 +22,7 @@
>  #include <net/mac80211.h>
>  
>  #include "common.h"
> +#include "commands.h"
>  
>  int
>  _il_poll_bit(struct il_priv *il, u32 addr, u32 bits, u32 mask, int timeout)
> @@ -1766,6 +1767,11 @@ il_process_add_sta_resp(struct il_priv *il, struct il_addsta_cmd *addsta,
>  		IL_ERR("Bad return from C_ADD_STA (0x%08X)\n", pkt->hdr.flags);
>  		return ret;
>  	}
> +	
> +	if (sta_id >= IL_STATION_COUNT) {
> +		IL_ERR(il, "invalid sta_id %u", sta_id);

Again, compile test your patches!

Regards
Stanislaw

> +		return -EINVAL;
> +	}
>  
>  	D_INFO("Processing response for adding station %u\n", sta_id);
>  
> -- 
> 2.34.1
>