lkml.org 
[lkml]   [2022]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v6 1/2] ath9k: fix use-after-free in ath9k_hif_usb_rx_cb
    Date
    Takashi Iwai <tiwai@suse.de> writes:

    > On Wed, 15 Jun 2022 11:05:20 +0200,
    > Toke Høiland-Jørgensen wrote:
    >
    >>
    >> Kalle Valo <kvalo@kernel.org> writes:
    >>
    >> > Toke Høiland-Jørgensen <toke@toke.dk> writes:
    >> >
    >> >> Pavel Skripkin <paskripkin@gmail.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@syzkaller.appspotmail.com
    >> >>> Reported-and-tested-by: syzbot+c6dde1f690b60e0b9fbe@syzkaller.appspotmail.com
    >> >>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.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@toke.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?
    >
    > Well, it does matter if it really breaks things, of course ;)
    >
    >> 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 :)
    >
    > Queuing to for-next is fine for us. Backporting immediately or not
    > will be a decision by each distro, then.
    >
    > OTOH, if anyone has tested it beforehand on a real hardware and
    > confirmed, at least, that it works for normal cases (no error path),
    > that should suffice for -rc inclusion, too, IMO.

    Ok, I'll take these to -next then. I just don't like taking untested
    patches, having them -next gives us more time to fix any issues (or
    revert the patches).

    --
    https://patchwork.kernel.org/project/linux-wireless/list/

    https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

    \
     
     \ /
      Last update: 2022-06-20 10:54    [W:3.827 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site