* 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)
[not found] ` <be8949ff-7ac2-3570-05ec-342036a6f52e@phrozen.org>
@ 2019-06-18 11:06 ` Kalle Valo
2019-06-18 11:13 ` John Crispin
[not found] ` <20190618114732.GD18776@kadam>
1 sibling, 1 reply; 4+ 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] 4+ 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 ` [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) Kalle Valo
@ 2019-06-18 11:13 ` John Crispin
2019-06-18 12:50 ` Kalle Valo
0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ 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)
[not found] ` <20190618114732.GD18776@kadam>
@ 2019-06-18 14:10 ` John Crispin
0 siblings, 0 replies; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2019-06-18 14:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190618065329.GY1893@kadam>
[not found] ` <be8949ff-7ac2-3570-05ec-342036a6f52e@phrozen.org>
2019-06-18 11:06 ` [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) Kalle Valo
2019-06-18 11:13 ` John Crispin
2019-06-18 12:50 ` Kalle Valo
[not found] ` <20190618114732.GD18776@kadam>
2019-06-18 14:10 ` John Crispin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox