All of lore.kernel.org
 help / color / mirror / Atom feed
* [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
@ 2019-06-18  6:53 kbuild test robot
  2019-06-18  6:59 ` John Crispin
  0 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2019-06-18  6:53 UTC (permalink / raw)
  To: kbuild, John Crispin; +Cc: Kalle Valo, ath10k, kbuild-all, Dan Carpenter

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
head:   0f82fec5679664bb91d6c167fd1a146f113e4197
commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k: set additional values inside wmi_peer_assoc_complete_cmd

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)

Old smatch warnings:
drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs 12)

# https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
git remote update ath6kl
git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
vim +1274 drivers/net/wireless/ath/ath11k/mac.c

258bbf52 Kalle Valo   2019-02-05  1260  
258bbf52 Kalle Valo   2019-02-05  1261  static void ath11k_peer_assoc_h_he(struct ath11k *ar,
258bbf52 Kalle Valo   2019-02-05  1262  				   struct ieee80211_vif *vif,
258bbf52 Kalle Valo   2019-02-05  1263  				   struct ieee80211_sta *sta,
258bbf52 Kalle Valo   2019-02-05  1264  				   struct peer_assoc_params *arg)
258bbf52 Kalle Valo   2019-02-05  1265  {
17aca2d9 John Crispin 2019-06-03  1266  	const struct ieee80211_sta_he_cap *he_cap = &sta->he_cap;
3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
17aca2d9 John Crispin 2019-06-03  1268  
17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
17aca2d9 John Crispin 2019-06-03  1270  		return;
17aca2d9 John Crispin 2019-06-03  1271  
17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
17aca2d9 John Crispin 2019-06-03  1273  
17aca2d9 John Crispin 2019-06-03 @1274  	memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
17aca2d9 John Crispin 2019-06-03  1275  	       sizeof(arg->peer_he_cap_macinfo));

Smatch thinks these are different sizes...  I don't have a copy of
struct peer_assoc_params so I can't check.

17aca2d9 John Crispin 2019-06-03  1276  	memcpy(&arg->peer_he_cap_phyinfo, he_cap->he_cap_elem.phy_cap_info,
17aca2d9 John Crispin 2019-06-03  1277  	       sizeof(arg->peer_he_cap_phyinfo));
17aca2d9 John Crispin 2019-06-03  1278  	memcpy(&arg->peer_he_ops, &vif->bss_conf.he_operation,
17aca2d9 John Crispin 2019-06-03  1279  	       sizeof(arg->peer_he_ops));
cbdb3159 John Crispin 2019-06-17  1280  	arg->peer_he_cap_macinfo_internal = 0x0;
17aca2d9 John Crispin 2019-06-03  1281  
17aca2d9 John Crispin 2019-06-03  1282  	if (he_cap->he_cap_elem.phy_cap_info[6] &
17aca2d9 John Crispin 2019-06-03  1283  	    IEEE80211_HE_PHY_CAP6_PPE_THRESHOLD_PRESENT) {
17aca2d9 John Crispin 2019-06-03  1284  		int bit = 7;
17aca2d9 John Crispin 2019-06-03  1285  		int nss, ru;
17aca2d9 John Crispin 2019-06-03  1286  
17aca2d9 John Crispin 2019-06-03  1287  		arg->peer_ppet.numss_m1 = he_cap->ppe_thres[0] &
17aca2d9 John Crispin 2019-06-03  1288  					  IEEE80211_PPE_THRES_NSS_MASK;
17aca2d9 John Crispin 2019-06-03  1289  		arg->peer_ppet.ru_bit_mask =
17aca2d9 John Crispin 2019-06-03  1290  			(he_cap->ppe_thres[0] &
17aca2d9 John Crispin 2019-06-03  1291  			 IEEE80211_PPE_THRES_RU_INDEX_BITMASK_MASK) >>
17aca2d9 John Crispin 2019-06-03  1292  			IEEE80211_PPE_THRES_RU_INDEX_BITMASK_POS;
17aca2d9 John Crispin 2019-06-03  1293  
17aca2d9 John Crispin 2019-06-03  1294  		for (nss = 0; nss <= arg->peer_ppet.numss_m1; nss++) {
17aca2d9 John Crispin 2019-06-03  1295  			for (ru = 0; ru < 4; ru++) {
17aca2d9 John Crispin 2019-06-03  1296  				u32 val = 0;
17aca2d9 John Crispin 2019-06-03  1297  				int i;
17aca2d9 John Crispin 2019-06-03  1298  
17aca2d9 John Crispin 2019-06-03  1299  				if ((arg->peer_ppet.ru_bit_mask & BIT(ru)) == 0)
17aca2d9 John Crispin 2019-06-03  1300  					continue;
17aca2d9 John Crispin 2019-06-03  1301  				for (i = 0; i < 6; i++) {
17aca2d9 John Crispin 2019-06-03  1302  					val >>= 1;
17aca2d9 John Crispin 2019-06-03  1303  					val |= ((he_cap->ppe_thres[bit / 8] >>
17aca2d9 John Crispin 2019-06-03  1304  						 (bit % 8)) & 0x1) << 5;
17aca2d9 John Crispin 2019-06-03  1305  					bit++;
17aca2d9 John Crispin 2019-06-03  1306  				}
17aca2d9 John Crispin 2019-06-03  1307  				arg->peer_ppet.ppet16_ppet8_ru3_ru0[nss] |=
17aca2d9 John Crispin 2019-06-03  1308  								val << (ru * 6);
17aca2d9 John Crispin 2019-06-03  1309  			}
17aca2d9 John Crispin 2019-06-03  1310  		}
17aca2d9 John Crispin 2019-06-03  1311  	}
17aca2d9 John Crispin 2019-06-03  1312  
17aca2d9 John Crispin 2019-06-03  1313  	switch (sta->bandwidth) {
17aca2d9 John Crispin 2019-06-03  1314  	case IEEE80211_STA_RX_BW_160:
17aca2d9 John Crispin 2019-06-03  1315  		if (he_cap->he_cap_elem.phy_cap_info[0] &
17aca2d9 John Crispin 2019-06-03  1316  		    IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G) {
3db59a23 Kalle Valo   2019-06-12  1317  			v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_80p80);
3db59a23 Kalle Valo   2019-06-12  1318  			arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80_80] = v;
3db59a23 Kalle Valo   2019-06-12  1319  
3db59a23 Kalle Valo   2019-06-12  1320  			v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_80p80);
3db59a23 Kalle Valo   2019-06-12  1321  			arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80_80] = v;
3db59a23 Kalle Valo   2019-06-12  1322  
17aca2d9 John Crispin 2019-06-03  1323  			arg->peer_he_mcs_count++;
17aca2d9 John Crispin 2019-06-03  1324  		}
3db59a23 Kalle Valo   2019-06-12  1325  
3db59a23 Kalle Valo   2019-06-12  1326  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_160);
3db59a23 Kalle Valo   2019-06-12  1327  		arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_160] = v;
3db59a23 Kalle Valo   2019-06-12  1328  
3db59a23 Kalle Valo   2019-06-12  1329  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_160);
3db59a23 Kalle Valo   2019-06-12  1330  		arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_160] = v;
3db59a23 Kalle Valo   2019-06-12  1331  
17aca2d9 John Crispin 2019-06-03  1332  		arg->peer_he_mcs_count++;
17aca2d9 John Crispin 2019-06-03  1333  		/* drop through */
17aca2d9 John Crispin 2019-06-03  1334  
17aca2d9 John Crispin 2019-06-03  1335  	default:
3db59a23 Kalle Valo   2019-06-12  1336  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_80);
3db59a23 Kalle Valo   2019-06-12  1337  		arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80] = v;
3db59a23 Kalle Valo   2019-06-12  1338  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_80);
3db59a23 Kalle Valo   2019-06-12  1339  		arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80] = v;
17aca2d9 John Crispin 2019-06-03  1340  		arg->peer_he_mcs_count++;
17aca2d9 John Crispin 2019-06-03  1341  		break;
17aca2d9 John Crispin 2019-06-03  1342  	}
258bbf52 Kalle Valo   2019-02-05  1343  }
258bbf52 Kalle Valo   2019-02-05  1344  

:::::: The code at line 1274 was first introduced by commit
:::::: 17aca2d9a969788a7f1e3e0c72b5485bf6a432a4 ath11k: add HE support

:::::: TO: John Crispin <john@phrozen.org>
:::::: CC: Kalle Valo <kvalo@codeaurora.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18  6:53 [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8) kbuild test robot
@ 2019-06-18  6:59 ` John Crispin
  2019-06-18 11:06   ` Kalle Valo
  2019-06-18 11:47   ` Dan Carpenter
  0 siblings, 2 replies; 7+ messages in thread
From: John Crispin @ 2019-06-18  6:59 UTC (permalink / raw)
  To: kbuild test robot, kbuild; +Cc: Kalle Valo, ath10k, kbuild-all, Dan Carpenter


On 18/06/2019 08:53, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
> head:   0f82fec5679664bb91d6c167fd1a146f113e4197
> commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k: set additional values inside wmi_peer_assoc_complete_cmd
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
>
> Old smatch warnings:
> drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs 12)
>
> # https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
> git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
> git remote update ath6kl
> git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
> vim +1274 drivers/net/wireless/ath/ath11k/mac.c
>
> 258bbf52 Kalle Valo   2019-02-05  1260
> 258bbf52 Kalle Valo   2019-02-05  1261  static void ath11k_peer_assoc_h_he(struct ath11k *ar,
> 258bbf52 Kalle Valo   2019-02-05  1262  				   struct ieee80211_vif *vif,
> 258bbf52 Kalle Valo   2019-02-05  1263  				   struct ieee80211_sta *sta,
> 258bbf52 Kalle Valo   2019-02-05  1264  				   struct peer_assoc_params *arg)
> 258bbf52 Kalle Valo   2019-02-05  1265  {
> 17aca2d9 John Crispin 2019-06-03  1266  	const struct ieee80211_sta_he_cap *he_cap = &sta->he_cap;
> 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
> 17aca2d9 John Crispin 2019-06-03  1268
> 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
> 17aca2d9 John Crispin 2019-06-03  1270  		return;
> 17aca2d9 John Crispin 2019-06-03  1271
> 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
> 17aca2d9 John Crispin 2019-06-03  1273
> 17aca2d9 John Crispin 2019-06-03 @1274  	memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
> 17aca2d9 John Crispin 2019-06-03  1275  	       sizeof(arg->peer_he_cap_macinfo));
>
> Smatch thinks these are different sizes...  I don't have a copy of
> struct peer_assoc_params so I can't check.

Hi,

its he_cap->he_cap_elem.mac_cap_info[6] and arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as the FW only cares for the first 2 bytes.

	John




>
> 17aca2d9 John Crispin 2019-06-03  1276  	memcpy(&arg->peer_he_cap_phyinfo, he_cap->he_cap_elem.phy_cap_info,
> 17aca2d9 John Crispin 2019-06-03  1277  	       sizeof(arg->peer_he_cap_phyinfo));
> 17aca2d9 John Crispin 2019-06-03  1278  	memcpy(&arg->peer_he_ops, &vif->bss_conf.he_operation,
> 17aca2d9 John Crispin 2019-06-03  1279  	       sizeof(arg->peer_he_ops));
> cbdb3159 John Crispin 2019-06-17  1280  	arg->peer_he_cap_macinfo_internal = 0x0;
> 17aca2d9 John Crispin 2019-06-03  1281
> 17aca2d9 John Crispin 2019-06-03  1282  	if (he_cap->he_cap_elem.phy_cap_info[6] &
> 17aca2d9 John Crispin 2019-06-03  1283  	    IEEE80211_HE_PHY_CAP6_PPE_THRESHOLD_PRESENT) {
> 17aca2d9 John Crispin 2019-06-03  1284  		int bit = 7;
> 17aca2d9 John Crispin 2019-06-03  1285  		int nss, ru;
> 17aca2d9 John Crispin 2019-06-03  1286
> 17aca2d9 John Crispin 2019-06-03  1287  		arg->peer_ppet.numss_m1 = he_cap->ppe_thres[0] &
> 17aca2d9 John Crispin 2019-06-03  1288  					  IEEE80211_PPE_THRES_NSS_MASK;
> 17aca2d9 John Crispin 2019-06-03  1289  		arg->peer_ppet.ru_bit_mask =
> 17aca2d9 John Crispin 2019-06-03  1290  			(he_cap->ppe_thres[0] &
> 17aca2d9 John Crispin 2019-06-03  1291  			 IEEE80211_PPE_THRES_RU_INDEX_BITMASK_MASK) >>
> 17aca2d9 John Crispin 2019-06-03  1292  			IEEE80211_PPE_THRES_RU_INDEX_BITMASK_POS;
> 17aca2d9 John Crispin 2019-06-03  1293
> 17aca2d9 John Crispin 2019-06-03  1294  		for (nss = 0; nss <= arg->peer_ppet.numss_m1; nss++) {
> 17aca2d9 John Crispin 2019-06-03  1295  			for (ru = 0; ru < 4; ru++) {
> 17aca2d9 John Crispin 2019-06-03  1296  				u32 val = 0;
> 17aca2d9 John Crispin 2019-06-03  1297  				int i;
> 17aca2d9 John Crispin 2019-06-03  1298
> 17aca2d9 John Crispin 2019-06-03  1299  				if ((arg->peer_ppet.ru_bit_mask & BIT(ru)) == 0)
> 17aca2d9 John Crispin 2019-06-03  1300  					continue;
> 17aca2d9 John Crispin 2019-06-03  1301  				for (i = 0; i < 6; i++) {
> 17aca2d9 John Crispin 2019-06-03  1302  					val >>= 1;
> 17aca2d9 John Crispin 2019-06-03  1303  					val |= ((he_cap->ppe_thres[bit / 8] >>
> 17aca2d9 John Crispin 2019-06-03  1304  						 (bit % 8)) & 0x1) << 5;
> 17aca2d9 John Crispin 2019-06-03  1305  					bit++;
> 17aca2d9 John Crispin 2019-06-03  1306  				}
> 17aca2d9 John Crispin 2019-06-03  1307  				arg->peer_ppet.ppet16_ppet8_ru3_ru0[nss] |=
> 17aca2d9 John Crispin 2019-06-03  1308  								val << (ru * 6);
> 17aca2d9 John Crispin 2019-06-03  1309  			}
> 17aca2d9 John Crispin 2019-06-03  1310  		}
> 17aca2d9 John Crispin 2019-06-03  1311  	}
> 17aca2d9 John Crispin 2019-06-03  1312
> 17aca2d9 John Crispin 2019-06-03  1313  	switch (sta->bandwidth) {
> 17aca2d9 John Crispin 2019-06-03  1314  	case IEEE80211_STA_RX_BW_160:
> 17aca2d9 John Crispin 2019-06-03  1315  		if (he_cap->he_cap_elem.phy_cap_info[0] &
> 17aca2d9 John Crispin 2019-06-03  1316  		    IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G) {
> 3db59a23 Kalle Valo   2019-06-12  1317  			v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_80p80);
> 3db59a23 Kalle Valo   2019-06-12  1318  			arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80_80] = v;
> 3db59a23 Kalle Valo   2019-06-12  1319
> 3db59a23 Kalle Valo   2019-06-12  1320  			v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_80p80);
> 3db59a23 Kalle Valo   2019-06-12  1321  			arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80_80] = v;
> 3db59a23 Kalle Valo   2019-06-12  1322
> 17aca2d9 John Crispin 2019-06-03  1323  			arg->peer_he_mcs_count++;
> 17aca2d9 John Crispin 2019-06-03  1324  		}
> 3db59a23 Kalle Valo   2019-06-12  1325
> 3db59a23 Kalle Valo   2019-06-12  1326  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_160);
> 3db59a23 Kalle Valo   2019-06-12  1327  		arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_160] = v;
> 3db59a23 Kalle Valo   2019-06-12  1328
> 3db59a23 Kalle Valo   2019-06-12  1329  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_160);
> 3db59a23 Kalle Valo   2019-06-12  1330  		arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_160] = v;
> 3db59a23 Kalle Valo   2019-06-12  1331
> 17aca2d9 John Crispin 2019-06-03  1332  		arg->peer_he_mcs_count++;
> 17aca2d9 John Crispin 2019-06-03  1333  		/* drop through */
> 17aca2d9 John Crispin 2019-06-03  1334
> 17aca2d9 John Crispin 2019-06-03  1335  	default:
> 3db59a23 Kalle Valo   2019-06-12  1336  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.rx_mcs_80);
> 3db59a23 Kalle Valo   2019-06-12  1337  		arg->peer_he_rx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80] = v;
> 3db59a23 Kalle Valo   2019-06-12  1338  		v = le16_to_cpu(he_cap->he_mcs_nss_supp.tx_mcs_80);
> 3db59a23 Kalle Valo   2019-06-12  1339  		arg->peer_he_tx_mcs_set[WMI_HECAP_TXRX_MCS_NSS_IDX_80] = v;
> 17aca2d9 John Crispin 2019-06-03  1340  		arg->peer_he_mcs_count++;
> 17aca2d9 John Crispin 2019-06-03  1341  		break;
> 17aca2d9 John Crispin 2019-06-03  1342  	}
> 258bbf52 Kalle Valo   2019-02-05  1343  }
> 258bbf52 Kalle Valo   2019-02-05  1344
>
> :::::: The code at line 1274 was first introduced by commit
> :::::: 17aca2d9a969788a7f1e3e0c72b5485bf6a432a4 ath11k: add HE support
>
> :::::: TO: John Crispin <john@phrozen.org>
> :::::: CC: Kalle Valo <kvalo@codeaurora.org>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18  6:59 ` John Crispin
@ 2019-06-18 11:06   ` Kalle Valo
  2019-06-18 11:13     ` John Crispin
  2019-06-18 11:47   ` Dan Carpenter
  1 sibling, 1 reply; 7+ messages in thread
From: Kalle Valo @ 2019-06-18 11:06 UTC (permalink / raw)
  To: John Crispin; +Cc: kbuild, ath11k, kbuild test robot, Dan Carpenter, kbuild-all

(moving from ath10k to ath11k list)

John Crispin <john@phrozen.org> writes:

> On 18/06/2019 08:53, kbuild test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
>> head:   0f82fec5679664bb91d6c167fd1a146f113e4197
>> commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k:
>> set additional values inside wmi_peer_assoc_complete_cmd
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> New smatch warnings:
>> drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he()
>> error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs
>> 8)
>>
>> Old smatch warnings:
>> drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he()
>> error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs
>> 12)
>>
>> #
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>> git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>> git remote update ath6kl
>> git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>> vim +1274 drivers/net/wireless/ath/ath11k/mac.c
>>
>> 258bbf52 Kalle Valo   2019-02-05  1260
>> 258bbf52 Kalle Valo 2019-02-05 1261 static void
>> ath11k_peer_assoc_h_he(struct ath11k *ar,
>> 258bbf52 Kalle Valo 2019-02-05 1262 struct ieee80211_vif *vif,
>> 258bbf52 Kalle Valo 2019-02-05 1263 struct ieee80211_sta *sta,
>> 258bbf52 Kalle Valo 2019-02-05 1264 struct peer_assoc_params *arg)
>> 258bbf52 Kalle Valo   2019-02-05  1265  {
>> 17aca2d9 John Crispin 2019-06-03 1266 const struct
>> ieee80211_sta_he_cap *he_cap = &sta->he_cap;
>> 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
>> 17aca2d9 John Crispin 2019-06-03  1268
>> 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
>> 17aca2d9 John Crispin 2019-06-03  1270  		return;
>> 17aca2d9 John Crispin 2019-06-03  1271
>> 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
>> 17aca2d9 John Crispin 2019-06-03  1273
>> 17aca2d9 John Crispin 2019-06-03 @1274
>> memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
>> 17aca2d9 John Crispin 2019-06-03 1275
>> sizeof(arg->peer_he_cap_macinfo));
>>
>> Smatch thinks these are different sizes...  I don't have a copy of
>> struct peer_assoc_params so I can't check.
>
> Hi,
>
> its he_cap->he_cap_elem.mac_cap_info[6] and
> arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as
> the FW only cares for the first 2 bytes.

Are the remaining bytes to zero? (ie. does it follow "the reserved
fields should be zero" rule). In general this makes it safer to have
changes in firmware interface in the future.

-- 
Kalle Valo

_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18 11:06   ` Kalle Valo
@ 2019-06-18 11:13     ` John Crispin
  2019-06-18 12:50       ` Kalle Valo
  0 siblings, 1 reply; 7+ messages in thread
From: John Crispin @ 2019-06-18 11:13 UTC (permalink / raw)
  To: ath11k


On 18/06/2019 13:06, Kalle Valo wrote:
> (moving from ath10k to ath11k list)
>
> John Crispin <john@phrozen.org> writes:
>
>> On 18/06/2019 08:53, kbuild test robot wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
>>> head:   0f82fec5679664bb91d6c167fd1a146f113e4197
>>> commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k:
>>> set additional values inside wmi_peer_assoc_complete_cmd
>>>
>>> If you fix the issue, kindly add following tag
>>> Reported-by: kbuild test robot <lkp@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> New smatch warnings:
>>> drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he()
>>> error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs
>>> 8)
>>>
>>> Old smatch warnings:
>>> drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he()
>>> error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs
>>> 12)
>>>
>>> #
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>> git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>>> git remote update ath6kl
>>> git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>> vim +1274 drivers/net/wireless/ath/ath11k/mac.c
>>>
>>> 258bbf52 Kalle Valo   2019-02-05  1260
>>> 258bbf52 Kalle Valo 2019-02-05 1261 static void
>>> ath11k_peer_assoc_h_he(struct ath11k *ar,
>>> 258bbf52 Kalle Valo 2019-02-05 1262 struct ieee80211_vif *vif,
>>> 258bbf52 Kalle Valo 2019-02-05 1263 struct ieee80211_sta *sta,
>>> 258bbf52 Kalle Valo 2019-02-05 1264 struct peer_assoc_params *arg)
>>> 258bbf52 Kalle Valo   2019-02-05  1265  {
>>> 17aca2d9 John Crispin 2019-06-03 1266 const struct
>>> ieee80211_sta_he_cap *he_cap = &sta->he_cap;
>>> 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
>>> 17aca2d9 John Crispin 2019-06-03  1268
>>> 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
>>> 17aca2d9 John Crispin 2019-06-03  1270  		return;
>>> 17aca2d9 John Crispin 2019-06-03  1271
>>> 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
>>> 17aca2d9 John Crispin 2019-06-03  1273
>>> 17aca2d9 John Crispin 2019-06-03 @1274
>>> memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
>>> 17aca2d9 John Crispin 2019-06-03 1275
>>> sizeof(arg->peer_he_cap_macinfo));
>>>
>>> Smatch thinks these are different sizes...  I don't have a copy of
>>> struct peer_assoc_params so I can't check.
>> Hi,
>>
>> its he_cap->he_cap_elem.mac_cap_info[6] and
>> arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as
>> the FW only cares for the first 2 bytes.
> Are the remaining bytes to zero? (ie. does it follow "the reserved
> fields should be zero" rule). In general this makes it safer to have
> changes in firmware interface in the future.


We copy the first 2 bytes of a 6 byte buffer into a 2 byte buffer. these 
2 bytes are later copied into 2 non-consecutive fields inside a WMI cmd




_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18  6:59 ` John Crispin
  2019-06-18 11:06   ` Kalle Valo
@ 2019-06-18 11:47   ` Dan Carpenter
  2019-06-18 14:10     ` John Crispin
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2019-06-18 11:47 UTC (permalink / raw)
  To: John Crispin; +Cc: Kalle Valo, kbuild, kbuild test robot, ath10k, kbuild-all

On Tue, Jun 18, 2019 at 08:59:55AM +0200, John Crispin wrote:
> 
> On 18/06/2019 08:53, kbuild test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
> > head:   0f82fec5679664bb91d6c167fd1a146f113e4197
> > commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k: set additional values inside wmi_peer_assoc_complete_cmd
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > New smatch warnings:
> > drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
> > 
> > Old smatch warnings:
> > drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs 12)
> > 
> > # https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
> > git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
> > git remote update ath6kl
> > git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
> > vim +1274 drivers/net/wireless/ath/ath11k/mac.c
> > 
> > 258bbf52 Kalle Valo   2019-02-05  1260
> > 258bbf52 Kalle Valo   2019-02-05  1261  static void ath11k_peer_assoc_h_he(struct ath11k *ar,
> > 258bbf52 Kalle Valo   2019-02-05  1262  				   struct ieee80211_vif *vif,
> > 258bbf52 Kalle Valo   2019-02-05  1263  				   struct ieee80211_sta *sta,
> > 258bbf52 Kalle Valo   2019-02-05  1264  				   struct peer_assoc_params *arg)
> > 258bbf52 Kalle Valo   2019-02-05  1265  {
> > 17aca2d9 John Crispin 2019-06-03  1266  	const struct ieee80211_sta_he_cap *he_cap = &sta->he_cap;
> > 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
> > 17aca2d9 John Crispin 2019-06-03  1268
> > 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
> > 17aca2d9 John Crispin 2019-06-03  1270  		return;
> > 17aca2d9 John Crispin 2019-06-03  1271
> > 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
> > 17aca2d9 John Crispin 2019-06-03  1273
> > 17aca2d9 John Crispin 2019-06-03 @1274  	memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
> > 17aca2d9 John Crispin 2019-06-03  1275  	       sizeof(arg->peer_he_cap_macinfo));
> > 
> > Smatch thinks these are different sizes...  I don't have a copy of
> > struct peer_assoc_params so I can't check.
> 
> Hi,
> 
> its he_cap->he_cap_elem.mac_cap_info[6] and arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as the FW only cares for the first 2 bytes.
> 

I did download the latest git.  The problem is that
he_cap->he_cap_elem.mac_cap_info[] is six bytes and arg->peer_he_cap_macinfo[]
is two u32s or eight bytes.  So we are reading beyond the end of the
array.

regards,
dan carpenter


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18 11:13     ` John Crispin
@ 2019-06-18 12:50       ` Kalle Valo
  0 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-06-18 12:50 UTC (permalink / raw)
  To: John Crispin; +Cc: ath11k

John Crispin <john@phrozen.org> writes:

> On 18/06/2019 13:06, Kalle Valo wrote:
>> (moving from ath10k to ath11k list)
>>
>> John Crispin <john@phrozen.org> writes:
>>
>>> On 18/06/2019 08:53, kbuild test robot wrote:
>>>> tree:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>>>> pending-ath11k
>>>> head:   0f82fec5679664bb91d6c167fd1a146f113e4197
>>>> commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k:
>>>> set additional values inside wmi_peer_assoc_complete_cmd
>>>>
>>>> If you fix the issue, kindly add following tag
>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>>
>>>> New smatch warnings:
>>>> drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he()
>>>> error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs
>>>> 8)
>>>>
>>>> Old smatch warnings:
>>>> drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he()
>>>> error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs
>>>> 12)
>>>>
>>>> #
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>>> git remote add ath6kl
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>>>> git remote update ath6kl
>>>> git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>>> vim +1274 drivers/net/wireless/ath/ath11k/mac.c
>>>>
>>>> 258bbf52 Kalle Valo   2019-02-05  1260
>>>> 258bbf52 Kalle Valo 2019-02-05 1261 static void
>>>> ath11k_peer_assoc_h_he(struct ath11k *ar,
>>>> 258bbf52 Kalle Valo 2019-02-05 1262 struct ieee80211_vif *vif,
>>>> 258bbf52 Kalle Valo 2019-02-05 1263 struct ieee80211_sta *sta,
>>>> 258bbf52 Kalle Valo 2019-02-05 1264 struct peer_assoc_params *arg)
>>>> 258bbf52 Kalle Valo   2019-02-05  1265  {
>>>> 17aca2d9 John Crispin 2019-06-03 1266 const struct
>>>> ieee80211_sta_he_cap *he_cap = &sta->he_cap;
>>>> 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
>>>> 17aca2d9 John Crispin 2019-06-03  1268
>>>> 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
>>>> 17aca2d9 John Crispin 2019-06-03  1270  		return;
>>>> 17aca2d9 John Crispin 2019-06-03  1271
>>>> 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
>>>> 17aca2d9 John Crispin 2019-06-03  1273
>>>> 17aca2d9 John Crispin 2019-06-03 @1274
>>>> memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
>>>> 17aca2d9 John Crispin 2019-06-03 1275
>>>> sizeof(arg->peer_he_cap_macinfo));
>>>>
>>>> Smatch thinks these are different sizes...  I don't have a copy of
>>>> struct peer_assoc_params so I can't check.
>>> Hi,
>>>
>>> its he_cap->he_cap_elem.mac_cap_info[6] and
>>> arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as
>>> the FW only cares for the first 2 bytes.
>> Are the remaining bytes to zero? (ie. does it follow "the reserved
>> fields should be zero" rule). In general this makes it safer to have
>> changes in firmware interface in the future.
>
>
> We copy the first 2 bytes of a 6 byte buffer into a 2 byte buffer.
> these 2 bytes are later copied into 2 non-consecutive fields inside a
> WMI cmd

Ah, good.

-- 
Kalle Valo

_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
  2019-06-18 11:47   ` Dan Carpenter
@ 2019-06-18 14:10     ` John Crispin
  0 siblings, 0 replies; 7+ messages in thread
From: John Crispin @ 2019-06-18 14:10 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kbuild, ath11k, kbuild test robot, Kalle Valo, kbuild-all


On 18/06/2019 13:47, Dan Carpenter wrote:
> On Tue, Jun 18, 2019 at 08:59:55AM +0200, John Crispin wrote:
>> On 18/06/2019 08:53, kbuild test robot wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending-ath11k
>>> head:   0f82fec5679664bb91d6c167fd1a146f113e4197
>>> commit: cbdb3159fdf450b7b3999a06600aa0e1fb78383f [198/205] ath11k: set additional values inside wmi_peer_assoc_complete_cmd
>>>
>>> If you fix the issue, kindly add following tag
>>> Reported-by: kbuild test robot <lkp@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> New smatch warnings:
>>> drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8)
>>>
>>> Old smatch warnings:
>>> drivers/net/wireless/ath/ath11k/mac.c:1276 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.phy_cap_info' too small (11 vs 12)
>>>
>>> # https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>> git remote add ath6kl https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
>>> git remote update ath6kl
>>> git checkout cbdb3159fdf450b7b3999a06600aa0e1fb78383f
>>> vim +1274 drivers/net/wireless/ath/ath11k/mac.c
>>>
>>> 258bbf52 Kalle Valo   2019-02-05  1260
>>> 258bbf52 Kalle Valo   2019-02-05  1261  static void ath11k_peer_assoc_h_he(struct ath11k *ar,
>>> 258bbf52 Kalle Valo   2019-02-05  1262  				   struct ieee80211_vif *vif,
>>> 258bbf52 Kalle Valo   2019-02-05  1263  				   struct ieee80211_sta *sta,
>>> 258bbf52 Kalle Valo   2019-02-05  1264  				   struct peer_assoc_params *arg)
>>> 258bbf52 Kalle Valo   2019-02-05  1265  {
>>> 17aca2d9 John Crispin 2019-06-03  1266  	const struct ieee80211_sta_he_cap *he_cap = &sta->he_cap;
>>> 3db59a23 Kalle Valo   2019-06-12  1267  	u16 v;
>>> 17aca2d9 John Crispin 2019-06-03  1268
>>> 17aca2d9 John Crispin 2019-06-03  1269  	if (!he_cap->has_he)
>>> 17aca2d9 John Crispin 2019-06-03  1270  		return;
>>> 17aca2d9 John Crispin 2019-06-03  1271
>>> 17aca2d9 John Crispin 2019-06-03  1272  	arg->he_flag = true;
>>> 17aca2d9 John Crispin 2019-06-03  1273
>>> 17aca2d9 John Crispin 2019-06-03 @1274  	memcpy(&arg->peer_he_cap_macinfo, he_cap->he_cap_elem.mac_cap_info,
>>> 17aca2d9 John Crispin 2019-06-03  1275  	       sizeof(arg->peer_he_cap_macinfo));
>>>
>>> Smatch thinks these are different sizes...  I don't have a copy of
>>> struct peer_assoc_params so I can't check.
>> Hi,
>>
>> its he_cap->he_cap_elem.mac_cap_info[6] and arg->peer_he_cap_macinfo[2] and we only copy the first 2 elements as the FW only cares for the first 2 bytes.
>>
> I did download the latest git.  The problem is that
> he_cap->he_cap_elem.mac_cap_info[] is six bytes and arg->peer_he_cap_macinfo[]
> is two u32s or eight bytes.  So we are reading beyond the end of the
> array.
>
> regards,
> dan carpenter

correct, thanks for the catch, will send a fix to kalle

     John



_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-18 14:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-18  6:53 [ath6kl:pending-ath11k 198/205] drivers/net/wireless/ath/ath11k/mac.c:1274 ath11k_peer_assoc_h_he() error: memcpy() 'he_cap->he_cap_elem.mac_cap_info' too small (6 vs 8) kbuild test robot
2019-06-18  6:59 ` John Crispin
2019-06-18 11:06   ` Kalle Valo
2019-06-18 11:13     ` John Crispin
2019-06-18 12:50       ` Kalle Valo
2019-06-18 11:47   ` Dan Carpenter
2019-06-18 14:10     ` John Crispin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.