From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1A8FC43334 for ; Wed, 15 Jun 2022 09:05:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235819AbiFOJFY (ORCPT ); Wed, 15 Jun 2022 05:05:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44718 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230240AbiFOJFX (ORCPT ); Wed, 15 Jun 2022 05:05:23 -0400 Received: from mail.toke.dk (mail.toke.dk [IPv6:2a0c:4d80:42:2001::664]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7F0FB3980F; Wed, 15 Jun 2022 02:05:22 -0700 (PDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1655283921; bh=mZJkc3EtpYZjHnwrFglpnwJJ1Hj8zbOLiuV4Bh+jD24=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=rTztdNLj2oECJXYk09U7wcYFzV90dyQIpL+L/VaZhamY54SGd3n4uQL2C/WlNctWU 6ZXx0QFdfMO1/NTh4MN/MnQKB+Ijwx83f6Zqx9dXS7FMxjgJ+6AzP1bcNmn4gNmyOz AK7rP5M/PhBdHWEsNZdk69NrWn741jrwF+2RnSx53RBlZg4CYsnoHWu11V78zOARY5 uPMKw4cxbDj5LOiB+BJ0aTFwOPpNFN1p2uOKQFKCdbZ994eIdIXWG2qqjVVey2JnYS JYH2N9qa8D0uC4748Rfzkq74mox10F2ONbrWza91LMwlJfprFJw90ljJmdU1toN2jp pQZRQ4eCMDhwA== To: Kalle Valo Cc: Pavel Skripkin , davem@davemloft.net, kuba@kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+03110230a11411024147@syzkaller.appspotmail.com, syzbot+c6dde1f690b60e0b9fbe@syzkaller.appspotmail.com Subject: Re: [PATCH v6 1/2] ath9k: fix use-after-free in ath9k_hif_usb_rx_cb In-Reply-To: <87k09ipfl9.fsf@kernel.org> References: <87o7yvzf33.fsf@toke.dk> <87k09ipfl9.fsf@kernel.org> Date: Wed, 15 Jun 2022 11:05:20 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87tu8mxpnz.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org Kalle Valo writes: > Toke H=C3=B8iland-J=C3=B8rgensen writes: > >> Pavel Skripkin 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 =3D priv; */ >>> ath9k_htc_wait_for_target() <--- Failed >>> ieee80211_free_hw() <--- priv pointer is freed >>> >>> >>> ... >>> 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=3D6ead44e37afb6866ac0c7dd121= b4ce07cb665f60 [0] >>> Link: https://syzkaller.appspot.com/bug?id=3Db8101ffcec107c0567a0cd8acb= bacec91e9ee8de [1] >>> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") >>> Reported-and-tested-by: syzbot+03110230a11411024147@syzkaller.appspotma= il.com >>> Reported-and-tested-by: syzbot+c6dde1f690b60e0b9fbe@syzkaller.appspotma= il.com >>> Signed-off-by: Pavel Skripkin >> >> 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=C3=B8iland-J=C3=B8rgensen > > 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