netdev - Re: [PATCH v6 1/2] ath9k: fix use-after-free in ath9k_hif_usb_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87tu8mxpnz.fsf@toke.dk>
Date:   Wed, 15 Jun 2022 11:05:20 +0200
From:   Toke Høiland-Jørgensen <toke@...e.dk>
To:     Kalle Valo <kvalo@...nel.org>
Cc:     Pavel Skripkin <paskripkin@...il.com>, davem@...emloft.net,
        kuba@...nel.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzbot+03110230a11411024147@...kaller.appspotmail.com,
        syzbot+c6dde1f690b60e0b9fbe@...kaller.appspotmail.com
Subject: Re: [PATCH v6 1/2] ath9k: fix use-after-free in ath9k_hif_usb_rx_cb

Kalle Valo <kvalo@...nel.org> writes:

> Toke Høiland-Jørgensen <toke@...e.dk> writes:
>
>> Pavel Skripkin <paskripkin@...il.com> writes:
>>
>>> Syzbot reported use-after-free Read in ath9k_hif_usb_rx_cb() [0]. The
>>> problem was in incorrect htc_handle->drv_priv initialization.
>>>
>>> Probable call trace which can trigger use-after-free:
>>>
>>> ath9k_htc_probe_device()
>>>   /* htc_handle->drv_priv = priv; */
>>>   ath9k_htc_wait_for_target()      <--- Failed
>>>   ieee80211_free_hw()		   <--- priv pointer is freed
>>>
>>> <IRQ>
>>> ...
>>> ath9k_hif_usb_rx_cb()
>>>   ath9k_hif_usb_rx_stream()
>>>    RX_STAT_INC()		<--- htc_handle->drv_priv access
>>>
>>> In order to not add fancy protection for drv_priv we can move
>>> htc_handle->drv_priv initialization at the end of the
>>> ath9k_htc_probe_device() and add helper macro to make
>>> all *_STAT_* macros NULL safe, since syzbot has reported related NULL
>>> deref in that macros [1]
>>>
>>> Link: https://syzkaller.appspot.com/bug?id=6ead44e37afb6866ac0c7dd121b4ce07cb665f60 [0]
>>> Link: https://syzkaller.appspot.com/bug?id=b8101ffcec107c0567a0cd8acbbacec91e9ee8de [1]
>>> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
>>> Reported-and-tested-by: syzbot+03110230a11411024147@...kaller.appspotmail.com
>>> Reported-and-tested-by: syzbot+c6dde1f690b60e0b9fbe@...kaller.appspotmail.com
>>> Signed-off-by: Pavel Skripkin <paskripkin@...il.com>
>>
>> Alright, since we've heard no more objections and the status quo is
>> definitely broken, let's get this merged and we can follow up with any
>> other fixes as necessary...
>>
>> Acked-by: Toke Høiland-Jørgensen <toke@...e.dk>
>
> I'm wondering should these go to -rc or -next? Has anyone actually
> tested these with real hardware? (syzbot testing does not count) With
> the past bad experience with syzbot fixes I'm leaning towards -next to
> have more time to fix any regressions.

Hmm, good question. From Takashi's comment on v5, it seems like distros
are going to backport it anyway, so in that sense it probably doesn't
matter that much?

In any case I think it has a fairly low probability of breaking real
users' setup (how often is that error path on setup even hit?), but I'm
OK with it going to -next to be doubleplus-sure :)

-Toke