diff mbox series

wifi: ath12k: fix reusing outside iterator in ath12k_wow_vif_set_wakeups()

Message ID 20240722033332.6273-1-quic_bqiang@quicinc.com
State New
Headers show
Series wifi: ath12k: fix reusing outside iterator in ath12k_wow_vif_set_wakeups() | expand

Commit Message

Baochen Qiang July 22, 2024, 3:33 a.m. UTC
Smatch throws below warning:

	drivers/net/wireless/ath/ath12k/wow.c:434 ath12k_wow_vif_set_wakeups()
	warn: reusing outside iterator: 'i'

	drivers/net/wireless/ath/ath12k/wow.c
	    411         default:
	    412                 break;
	    413         }
	    414
	    415         for (i = 0; i < wowlan->n_patterns; i++) {
	                            ^^^^^^^^^^^^^^^^^^^^^^
	Here we loop until ->n_patterns

	    416                 const struct cfg80211_pkt_pattern *eth_pattern = &patterns[i];
	    417                 struct ath12k_pkt_pattern new_pattern = {};
	    418
	    419                 if (WARN_ON(eth_pattern->pattern_len > WOW_MAX_PATTERN_SIZE))
	    420                         return -EINVAL;
	    421
	    422                 if (ar->ab->wow.wmi_conf_rx_decap_mode ==
	    423                     ATH12K_HW_TXRX_NATIVE_WIFI) {
	    424                         ath12k_wow_convert_8023_to_80211(ar, eth_pattern,
	    425                                                          &new_pattern);
	    426
	    427                         if (WARN_ON(new_pattern.pattern_len > WOW_MAX_PATTERN_SIZE))
	    428                                 return -EINVAL;
	    429                 } else {
	    430                         memcpy(new_pattern.pattern, eth_pattern->pattern,
	    431                                eth_pattern->pattern_len);
	    432
	    433                         /* convert bitmask to bytemask */
	--> 434                         for (i = 0; i < eth_pattern->pattern_len; i++)
	    435                                 if (eth_pattern->mask[i / 8] & BIT(i % 8))
	    436                                         new_pattern.bytemask[i] = 0xff;

	This loop re-uses i and the loop ends with i == eth_pattern->pattern_len.
	This looks like a bug.

Change to use a new iterator 'j' for the inner loop to fix it.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Fixes: 4a3c212eee0e ("wifi: ath12k: add basic WoW functionalities")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/d4975b95-9c43-45af-a0ab-80253f18c7f2@stanley.mountain/
Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/wow.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


base-commit: db1ce56e6e1d395dd42a3cd6332a871d9be59c45

Comments

Jeff Johnson July 22, 2024, 11:55 p.m. UTC | #1
On 7/21/2024 8:33 PM, Baochen Qiang wrote:
> Smatch throws below warning:
> 
> 	drivers/net/wireless/ath/ath12k/wow.c:434 ath12k_wow_vif_set_wakeups()
> 	warn: reusing outside iterator: 'i'
> 
> 	drivers/net/wireless/ath/ath12k/wow.c
> 	    411         default:
> 	    412                 break;
> 	    413         }
> 	    414
> 	    415         for (i = 0; i < wowlan->n_patterns; i++) {
> 	                            ^^^^^^^^^^^^^^^^^^^^^^
> 	Here we loop until ->n_patterns
> 
> 	    416                 const struct cfg80211_pkt_pattern *eth_pattern = &patterns[i];
> 	    417                 struct ath12k_pkt_pattern new_pattern = {};
> 	    418
> 	    419                 if (WARN_ON(eth_pattern->pattern_len > WOW_MAX_PATTERN_SIZE))
> 	    420                         return -EINVAL;
> 	    421
> 	    422                 if (ar->ab->wow.wmi_conf_rx_decap_mode ==
> 	    423                     ATH12K_HW_TXRX_NATIVE_WIFI) {
> 	    424                         ath12k_wow_convert_8023_to_80211(ar, eth_pattern,
> 	    425                                                          &new_pattern);
> 	    426
> 	    427                         if (WARN_ON(new_pattern.pattern_len > WOW_MAX_PATTERN_SIZE))
> 	    428                                 return -EINVAL;
> 	    429                 } else {
> 	    430                         memcpy(new_pattern.pattern, eth_pattern->pattern,
> 	    431                                eth_pattern->pattern_len);
> 	    432
> 	    433                         /* convert bitmask to bytemask */
> 	--> 434                         for (i = 0; i < eth_pattern->pattern_len; i++)
> 	    435                                 if (eth_pattern->mask[i / 8] & BIT(i % 8))
> 	    436                                         new_pattern.bytemask[i] = 0xff;
> 
> 	This loop re-uses i and the loop ends with i == eth_pattern->pattern_len.
> 	This looks like a bug.
> 
> Change to use a new iterator 'j' for the inner loop to fix it.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Fixes: 4a3c212eee0e ("wifi: ath12k: add basic WoW functionalities")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/d4975b95-9c43-45af-a0ab-80253f18c7f2@stanley.mountain/
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
index c5cba825a84a..027f036eb415 100644
--- a/drivers/net/wireless/ath/ath12k/wow.c
+++ b/drivers/net/wireless/ath/ath12k/wow.c
@@ -361,7 +361,7 @@  static int ath12k_wow_vif_set_wakeups(struct ath12k_vif *arvif,
 	struct ath12k *ar = arvif->ar;
 	unsigned long wow_mask = 0;
 	int pattern_id = 0;
-	int ret, i;
+	int ret, i, j;
 
 	/* Setup requested WOW features */
 	switch (arvif->vdev_type) {
@@ -431,9 +431,9 @@  static int ath12k_wow_vif_set_wakeups(struct ath12k_vif *arvif,
 			       eth_pattern->pattern_len);
 
 			/* convert bitmask to bytemask */
-			for (i = 0; i < eth_pattern->pattern_len; i++)
-				if (eth_pattern->mask[i / 8] & BIT(i % 8))
-					new_pattern.bytemask[i] = 0xff;
+			for (j = 0; j < eth_pattern->pattern_len; j++)
+				if (eth_pattern->mask[j / 8] & BIT(j % 8))
+					new_pattern.bytemask[j] = 0xff;
 
 			new_pattern.pattern_len = eth_pattern->pattern_len;
 			new_pattern.pkt_offset = eth_pattern->pkt_offset;