wifi: ath9k: avoid uninit memory read in ath9k_htc_rx_msg()
[ Upstream commit b383e8abed41cc6ff1a3b34de75df9397fa4878c ] syzbot is reporting uninit value at ath9k_htc_rx_msg() [1], for ioctl(USB_RAW_IOCTL_EP_WRITE) can call ath9k_hif_usb_rx_stream() with pkt_len = 0 but ath9k_hif_usb_rx_stream() uses __dev_alloc_skb(pkt_len + 32, GFP_ATOMIC) based on an assumption that pkt_len is valid. As a result, ath9k_hif_usb_rx_stream() allocates skb with uninitialized memory and ath9k_htc_rx_msg() is reading from uninitialized memory. Since bytes accessed by ath9k_htc_rx_msg() is not known until ath9k_htc_rx_msg() is called, it would be difficult to check minimal valid pkt_len at "if (pkt_len > 2 * MAX_RX_BUF_SIZE) {" line in ath9k_hif_usb_rx_stream(). We have two choices. One is to workaround by adding __GFP_ZERO so that ath9k_htc_rx_msg() sees 0 if pkt_len is invalid. The other is to let ath9k_htc_rx_msg() validate pkt_len before accessing. This patch chose the latter. Note that I'm not sure threshold condition is correct, for I can't find details on possible packet length used by this protocol. Link: https://syzkaller.appspot.com/bug?extid=2ca247c2d60c7023de7f [1] Reported-by: syzbot <syzbot+2ca247c2d60c7023de7f@syzkaller.appspotmail.com> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Link: https://lore.kernel.org/r/7acfa1be-4b5c-b2ce-de43-95b0593fb3e5@I-love.SAKURA.ne.jp Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
42d579d910
commit
2c485f4f2a
@ -364,33 +364,27 @@ ret:
|
||||
}
|
||||
|
||||
static void ath9k_htc_fw_panic_report(struct htc_target *htc_handle,
|
||||
struct sk_buff *skb)
|
||||
struct sk_buff *skb, u32 len)
|
||||
{
|
||||
uint32_t *pattern = (uint32_t *)skb->data;
|
||||
|
||||
switch (*pattern) {
|
||||
case 0x33221199:
|
||||
{
|
||||
if (*pattern == 0x33221199 && len >= sizeof(struct htc_panic_bad_vaddr)) {
|
||||
struct htc_panic_bad_vaddr *htc_panic;
|
||||
htc_panic = (struct htc_panic_bad_vaddr *) skb->data;
|
||||
dev_err(htc_handle->dev, "ath: firmware panic! "
|
||||
"exccause: 0x%08x; pc: 0x%08x; badvaddr: 0x%08x.\n",
|
||||
htc_panic->exccause, htc_panic->pc,
|
||||
htc_panic->badvaddr);
|
||||
break;
|
||||
}
|
||||
case 0x33221299:
|
||||
{
|
||||
return;
|
||||
}
|
||||
if (*pattern == 0x33221299) {
|
||||
struct htc_panic_bad_epid *htc_panic;
|
||||
htc_panic = (struct htc_panic_bad_epid *) skb->data;
|
||||
dev_err(htc_handle->dev, "ath: firmware panic! "
|
||||
"bad epid: 0x%08x\n", htc_panic->epid);
|
||||
break;
|
||||
}
|
||||
default:
|
||||
dev_err(htc_handle->dev, "ath: unknown panic pattern!\n");
|
||||
break;
|
||||
return;
|
||||
}
|
||||
dev_err(htc_handle->dev, "ath: unknown panic pattern!\n");
|
||||
}
|
||||
|
||||
/*
|
||||
@ -411,16 +405,26 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle,
|
||||
if (!htc_handle || !skb)
|
||||
return;
|
||||
|
||||
/* A valid message requires len >= 8.
|
||||
*
|
||||
* sizeof(struct htc_frame_hdr) == 8
|
||||
* sizeof(struct htc_ready_msg) == 8
|
||||
* sizeof(struct htc_panic_bad_vaddr) == 16
|
||||
* sizeof(struct htc_panic_bad_epid) == 8
|
||||
*/
|
||||
if (unlikely(len < sizeof(struct htc_frame_hdr)))
|
||||
goto invalid;
|
||||
htc_hdr = (struct htc_frame_hdr *) skb->data;
|
||||
epid = htc_hdr->endpoint_id;
|
||||
|
||||
if (epid == 0x99) {
|
||||
ath9k_htc_fw_panic_report(htc_handle, skb);
|
||||
ath9k_htc_fw_panic_report(htc_handle, skb, len);
|
||||
kfree_skb(skb);
|
||||
return;
|
||||
}
|
||||
|
||||
if (epid < 0 || epid >= ENDPOINT_MAX) {
|
||||
invalid:
|
||||
if (pipe_id != USB_REG_IN_PIPE)
|
||||
dev_kfree_skb_any(skb);
|
||||
else
|
||||
@ -432,21 +436,30 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle,
|
||||
|
||||
/* Handle trailer */
|
||||
if (htc_hdr->flags & HTC_FLAGS_RECV_TRAILER) {
|
||||
if (be32_to_cpu(*(__be32 *) skb->data) == 0x00C60000)
|
||||
if (be32_to_cpu(*(__be32 *) skb->data) == 0x00C60000) {
|
||||
/* Move past the Watchdog pattern */
|
||||
htc_hdr = (struct htc_frame_hdr *)(skb->data + 4);
|
||||
len -= 4;
|
||||
}
|
||||
}
|
||||
|
||||
/* Get the message ID */
|
||||
if (unlikely(len < sizeof(struct htc_frame_hdr) + sizeof(__be16)))
|
||||
goto invalid;
|
||||
msg_id = (__be16 *) ((void *) htc_hdr +
|
||||
sizeof(struct htc_frame_hdr));
|
||||
|
||||
/* Now process HTC messages */
|
||||
switch (be16_to_cpu(*msg_id)) {
|
||||
case HTC_MSG_READY_ID:
|
||||
if (unlikely(len < sizeof(struct htc_ready_msg)))
|
||||
goto invalid;
|
||||
htc_process_target_rdy(htc_handle, htc_hdr);
|
||||
break;
|
||||
case HTC_MSG_CONNECT_SERVICE_RESPONSE_ID:
|
||||
if (unlikely(len < sizeof(struct htc_frame_hdr) +
|
||||
sizeof(struct htc_conn_svc_rspmsg)))
|
||||
goto invalid;
|
||||
htc_process_conn_rsp(htc_handle, htc_hdr);
|
||||
break;
|
||||
default:
|
||||
|
Loading…
Reference in New Issue
Block a user