* [PATCH] ath9k_htc: fix uninit value bugs
@ 2022-01-15 12:27 Pavel Skripkin
2022-01-17 12:57 ` Kalle Valo
2022-01-28 12:32 ` Kalle Valo
0 siblings, 2 replies; 6+ messages in thread
From: Pavel Skripkin @ 2022-01-15 12:27 UTC (permalink / raw)
To: ath9k-devel, kvalo, davem, kuba, linville, vasanth,
Sujith.Manoharan, senthilkumar
Cc: linux-wireless, netdev, linux-kernel, Pavel Skripkin,
syzbot+f83a1df1ed4f67e8d8ad
Syzbot reported 2 KMSAN bugs in ath9k. All of them are caused by missing
field initialization.
In htc_connect_service() svc_meta_len and pad are not initialized. Based
on code it looks like in current skb there is no service data, so simply
initialize svc_meta_len to 0.
htc_issue_send() does not initialize htc_frame_hdr::control array. Based
on firmware code, it will initialize it by itself, so simply zero whole
array to make KMSAN happy
Fail logs:
BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
...
Uninit was created at:
slab_post_alloc_hook mm/slab.h:524 [inline]
slab_alloc_node mm/slub.c:3251 [inline]
__kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
kmalloc_reserve net/core/skbuff.c:354 [inline]
__alloc_skb+0x545/0xf90 net/core/skbuff.c:426
alloc_skb include/linux/skbuff.h:1126 [inline]
htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
...
Bytes 4-7 of 18 are uninitialized
Memory access of size 18 starts at ffff888027377e00
BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
...
Uninit was created at:
slab_post_alloc_hook mm/slab.h:524 [inline]
slab_alloc_node mm/slub.c:3251 [inline]
__kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
kmalloc_reserve net/core/skbuff.c:354 [inline]
__alloc_skb+0x545/0xf90 net/core/skbuff.c:426
alloc_skb include/linux/skbuff.h:1126 [inline]
htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
...
Bytes 16-17 of 18 are uninitialized
Memory access of size 18 starts at ffff888027377e00
Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/net/wireless/ath/ath9k/htc_hst.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index 510e61e97dbc..994ec48b2f66 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -30,6 +30,7 @@ static int htc_issue_send(struct htc_target *target, struct sk_buff* skb,
hdr->endpoint_id = epid;
hdr->flags = flags;
hdr->payload_len = cpu_to_be16(len);
+ memset(hdr->control, 0, sizeof(hdr->control));
status = target->hif->send(target->hif_dev, endpoint->ul_pipeid, skb);
@@ -272,6 +273,10 @@ int htc_connect_service(struct htc_target *target,
conn_msg->dl_pipeid = endpoint->dl_pipeid;
conn_msg->ul_pipeid = endpoint->ul_pipeid;
+ /* To prevent infoleak */
+ conn_msg->svc_meta_len = 0;
+ conn_msg->pad = 0;
+
ret = htc_issue_send(target, skb, skb->len, 0, ENDPOINT0);
if (ret)
goto err;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k_htc: fix uninit value bugs
2022-01-15 12:27 [PATCH] ath9k_htc: fix uninit value bugs Pavel Skripkin
@ 2022-01-17 12:57 ` Kalle Valo
2022-01-17 18:35 ` Pavel Skripkin
2022-01-28 12:32 ` Kalle Valo
1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2022-01-17 12:57 UTC (permalink / raw)
To: Pavel Skripkin
Cc: ath9k-devel, kvalo, davem, kuba, linville, vasanth,
Sujith.Manoharan, senthilkumar, linux-wireless, netdev,
linux-kernel, Pavel Skripkin, syzbot+f83a1df1ed4f67e8d8ad
Pavel Skripkin <paskripkin@gmail.com> wrote:
> Syzbot reported 2 KMSAN bugs in ath9k. All of them are caused by missing
> field initialization.
>
> In htc_connect_service() svc_meta_len and pad are not initialized. Based
> on code it looks like in current skb there is no service data, so simply
> initialize svc_meta_len to 0.
>
> htc_issue_send() does not initialize htc_frame_hdr::control array. Based
> on firmware code, it will initialize it by itself, so simply zero whole
> array to make KMSAN happy
>
> Fail logs:
>
> BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
> hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
> htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
> htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
> ...
>
> Uninit was created at:
> slab_post_alloc_hook mm/slab.h:524 [inline]
> slab_alloc_node mm/slub.c:3251 [inline]
> __kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
> kmalloc_reserve net/core/skbuff.c:354 [inline]
> __alloc_skb+0x545/0xf90 net/core/skbuff.c:426
> alloc_skb include/linux/skbuff.h:1126 [inline]
> htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
> ...
>
> Bytes 4-7 of 18 are uninitialized
> Memory access of size 18 starts at ffff888027377e00
>
> BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
> hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
> htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
> htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
> ...
>
> Uninit was created at:
> slab_post_alloc_hook mm/slab.h:524 [inline]
> slab_alloc_node mm/slub.c:3251 [inline]
> __kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
> kmalloc_reserve net/core/skbuff.c:354 [inline]
> __alloc_skb+0x545/0xf90 net/core/skbuff.c:426
> alloc_skb include/linux/skbuff.h:1126 [inline]
> htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
> ...
>
> Bytes 16-17 of 18 are uninitialized
> Memory access of size 18 starts at ffff888027377e00
>
> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
> Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
How did you test this? As syzbot is mentioned I assume you did not test this on
a real device, it would help a lot if this is clearly mentioned in the commit
log. My trust on syzbot fixes is close to zero due to bad past history.
--
https://patchwork.kernel.org/project/linux-wireless/patch/20220115122733.11160-1-paskripkin@gmail.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k_htc: fix uninit value bugs
2022-01-17 12:57 ` Kalle Valo
@ 2022-01-17 18:35 ` Pavel Skripkin
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Skripkin @ 2022-01-17 18:35 UTC (permalink / raw)
To: Kalle Valo
Cc: ath9k-devel, kvalo, davem, kuba, linville, vasanth,
Sujith.Manoharan, senthilkumar, linux-wireless, netdev,
linux-kernel, syzbot+f83a1df1ed4f67e8d8ad
Hi Kalle,
On 1/17/22 15:57, Kalle Valo wrote:
>> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
>> Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>
> How did you test this? As syzbot is mentioned I assume you did not test this on
> a real device, it would help a lot if this is clearly mentioned in the commit
> log. My trust on syzbot fixes is close to zero due to bad past history.
>
You are right, I've tested only with syzbot's reproducer. I've followed
simple guess: if code works properly with random values in these fields
for 14+ years, then zeroing them won't hurt much.
I might be missing something, but unfortunately I don't have suitable hw
piece to test the change.
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k_htc: fix uninit value bugs
2022-01-15 12:27 [PATCH] ath9k_htc: fix uninit value bugs Pavel Skripkin
2022-01-17 12:57 ` Kalle Valo
@ 2022-01-28 12:32 ` Kalle Valo
2022-01-28 20:52 ` Pavel Skripkin
1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2022-01-28 12:32 UTC (permalink / raw)
To: Pavel Skripkin
Cc: ath9k-devel, kvalo, davem, kuba, linville, vasanth,
Sujith.Manoharan, senthilkumar, linux-wireless, netdev,
linux-kernel, Pavel Skripkin, syzbot+f83a1df1ed4f67e8d8ad
Pavel Skripkin <paskripkin@gmail.com> wrote:
> Syzbot reported 2 KMSAN bugs in ath9k. All of them are caused by missing
> field initialization.
>
> In htc_connect_service() svc_meta_len and pad are not initialized. Based
> on code it looks like in current skb there is no service data, so simply
> initialize svc_meta_len to 0.
>
> htc_issue_send() does not initialize htc_frame_hdr::control array. Based
> on firmware code, it will initialize it by itself, so simply zero whole
> array to make KMSAN happy
>
> Fail logs:
>
> BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
> hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
> htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
> htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
> ...
>
> Uninit was created at:
> slab_post_alloc_hook mm/slab.h:524 [inline]
> slab_alloc_node mm/slub.c:3251 [inline]
> __kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
> kmalloc_reserve net/core/skbuff.c:354 [inline]
> __alloc_skb+0x545/0xf90 net/core/skbuff.c:426
> alloc_skb include/linux/skbuff.h:1126 [inline]
> htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
> ...
>
> Bytes 4-7 of 18 are uninitialized
> Memory access of size 18 starts at ffff888027377e00
>
> BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> usb_submit_urb+0x6c1/0x2aa0 drivers/usb/core/urb.c:430
> hif_usb_send_regout drivers/net/wireless/ath/ath9k/hif_usb.c:127 [inline]
> hif_usb_send+0x5f0/0x16f0 drivers/net/wireless/ath/ath9k/hif_usb.c:479
> htc_issue_send drivers/net/wireless/ath/ath9k/htc_hst.c:34 [inline]
> htc_connect_service+0x143e/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:275
> ...
>
> Uninit was created at:
> slab_post_alloc_hook mm/slab.h:524 [inline]
> slab_alloc_node mm/slub.c:3251 [inline]
> __kmalloc_node_track_caller+0xe0c/0x1510 mm/slub.c:4974
> kmalloc_reserve net/core/skbuff.c:354 [inline]
> __alloc_skb+0x545/0xf90 net/core/skbuff.c:426
> alloc_skb include/linux/skbuff.h:1126 [inline]
> htc_connect_service+0x1029/0x1960 drivers/net/wireless/ath/ath9k/htc_hst.c:258
> ...
>
> Bytes 16-17 of 18 are uninitialized
> Memory access of size 18 starts at ffff888027377e00
>
> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
> Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Patch applied to ath-next branch of ath.git, thanks.
d1e0df1c57bd ath9k_htc: fix uninit value bugs
--
https://patchwork.kernel.org/project/linux-wireless/patch/20220115122733.11160-1-paskripkin@gmail.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k_htc: fix uninit value bugs
2022-01-28 12:32 ` Kalle Valo
@ 2022-01-28 20:52 ` Pavel Skripkin
2022-02-03 8:39 ` Kalle Valo
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2022-01-28 20:52 UTC (permalink / raw)
To: Kalle Valo
Cc: ath9k-devel, kvalo, davem, kuba, linville, vasanth,
Sujith.Manoharan, senthilkumar, linux-wireless, netdev,
linux-kernel, syzbot+f83a1df1ed4f67e8d8ad
Hi Kalle,
On 1/28/22 15:32, Kalle Valo wrote:
>> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
>> Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>
> Patch applied to ath-next branch of ath.git, thanks.
>
> d1e0df1c57bd ath9k_htc: fix uninit value bugs
>
Thanks, Kalle! Can you also, please, check out this one too :)
Quite old, but syzbot is getting mad with this bug (like 20k hits). Thanks!
https://lore.kernel.org/all/20210922164204.32680-1-paskripkin@gmail.com/
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k_htc: fix uninit value bugs
2022-01-28 20:52 ` Pavel Skripkin
@ 2022-02-03 8:39 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2022-02-03 8:39 UTC (permalink / raw)
To: Pavel Skripkin
Cc: ath9k-devel, davem, kuba, linville, linux-wireless, netdev,
linux-kernel, syzbot+f83a1df1ed4f67e8d8ad,
Toke Høiland-Jørgensen
Pavel Skripkin <paskripkin@gmail.com> writes:
> Hi Kalle,
>
> On 1/28/22 15:32, Kalle Valo wrote:
>>> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
>>> Reported-by: syzbot+f83a1df1ed4f67e8d8ad@syzkaller.appspotmail.com
>>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>>
>> Patch applied to ath-next branch of ath.git, thanks.
>>
>> d1e0df1c57bd ath9k_htc: fix uninit value bugs
>>
>
> Thanks, Kalle! Can you also, please, check out this one too :)
> Quite old, but syzbot is getting mad with this bug (like 20k hits). Thanks!
>
>
> https://lore.kernel.org/all/20210922164204.32680-1-paskripkin@gmail.com/
I already provided feedback on August 6th:
"Separate patch for cleanups, please."
https://patchwork.kernel.org/project/linux-wireless/patch/20210804194841.14544-1-paskripkin@gmail.com/
Please submit v2. And Toke is the new ath9k maintainer, so please CC him
as well.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-03 8:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-15 12:27 [PATCH] ath9k_htc: fix uninit value bugs Pavel Skripkin
2022-01-17 12:57 ` Kalle Valo
2022-01-17 18:35 ` Pavel Skripkin
2022-01-28 12:32 ` Kalle Valo
2022-01-28 20:52 ` Pavel Skripkin
2022-02-03 8:39 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).