lkml.org 
[lkml]   [2022]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 310/390] wifi: ath9k: avoid uninit memory read in ath9k_htc_rx_msg()
    Date
    From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

    [ 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>
    ---
    drivers/net/wireless/ath/ath9k/htc_hst.c | 43 +++++++++++++++---------
    1 file changed, 28 insertions(+), 15 deletions(-)

    diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
    index 994ec48b2f66..ca05b07a45e6 100644
    --- a/drivers/net/wireless/ath/ath9k/htc_hst.c
    +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
    @@ -364,33 +364,27 @@ void ath9k_htc_txcompletion_cb(struct htc_target *htc_handle,
    }

    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:
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-10-24 18:02    [W:6.194 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site