From patchwork Mon Mar 27 09:24:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arend Van Spriel X-Patchwork-Id: 9645761 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 942B6602BF for ; Mon, 27 Mar 2017 09:25:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9475228236 for ; Mon, 27 Mar 2017 09:25:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8939928338; Mon, 27 Mar 2017 09:25:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BBFD928236 for ; Mon, 27 Mar 2017 09:25:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752088AbdC0JZj (ORCPT ); Mon, 27 Mar 2017 05:25:39 -0400 Received: from mail-qk0-f172.google.com ([209.85.220.172]:35899 "EHLO mail-qk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394AbdC0JZh (ORCPT ); Mon, 27 Mar 2017 05:25:37 -0400 Received: by mail-qk0-f172.google.com with SMTP id p22so32971122qka.3 for ; Mon, 27 Mar 2017 02:25:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=CrDYvPwilLkuHIagPFJVAt3V20wv7bGKTN7/Hq5/NMA=; b=hA1oDacZW1BwBlZRpndRhlJu58+gpJJ/7/G9rWrqbkLRVP2XYWAnu7YWfXG1WbRVxn 8RIzm1bKaUc89pYQ6SsSIHs8leNNaarsVLi81YS2OY0PK3F6FlSZqWqO8yevi9T8drEc jpzArCkPq4kUfTCMR5HO32mJh6n/HdLu++HS0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=CrDYvPwilLkuHIagPFJVAt3V20wv7bGKTN7/Hq5/NMA=; b=ktzDL6s2y1ZuzI5mwvxtuQEqNJlI1TKSy5btetkj/W627Jhm19zsL4LJjya1P/Ik+u zBJG6FQHLKg1t3qekyHi0woeybgRMXc+0BS9yWXNPJto4vpy+TjU+kxedcHnxgDwr0F6 CVI9QzPQtGSIWoUx7TdFdlJ7ULva7zb14T8DUwp4GUVcWw2w/zj3f04CiRaZ80Z3mfEr 1EkCmKaKmYvkvxHpiNYdbbEwrxv6FcbiT8KeQGd+eeq68+IrwrrJZEbQAimTeQ0CRjHG eehX+q5XxsHI6XSQosdM2LO3jj1w9iiFNWZY/bPWzgEPTB5B7Dkb/YSa1jzOXMjY949l VxvA== X-Gm-Message-State: AFeK/H08W0AzWcuKYTToTXPhMz7cBIq0Xg6CMkla4jPHPrFlAzJs3JkhUvELGSNNicaKYZytiDbWGlgo1Kkb5OXU X-Received: by 10.55.215.86 with SMTP id m83mr17918584qki.150.1490606698184; Mon, 27 Mar 2017 02:24:58 -0700 (PDT) MIME-Version: 1.0 Received: by 10.12.144.144 with HTTP; Mon, 27 Mar 2017 02:24:57 -0700 (PDT) In-Reply-To: References: From: Arend Van Spriel Date: Mon, 27 Mar 2017 11:24:57 +0200 Message-ID: Subject: Re: [4.9.13] brcmf use-after-free on resume To: Daniel J Blueman , Johannes Berg Cc: Netdev , linux-wireless , "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP + Johannes On 6-3-2017 11:48, Arend Van Spriel wrote: > + linux-wireless > > On 6-3-2017 8:04, Daniel J Blueman wrote: >> When resuming from suspend with a BCM43602 on Ubuntu 16.04 with >> 4.9.13, we see use after free [1]. >> >> We see the struct cfg80211_ops is accessed in the resume path, after >> it was previously freed: >> >> (gdb) list *(brcmf_cfg80211_attach+0x10b) >> 0x1d77b is in brcmf_cfg80211_attach >> (drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6861). >> 6856 brcmf_err("ndev is invalid\n"); >> 6857 return NULL; >> 6858 } >> 6859 >> 6860 ops = kmemdup(&brcmf_cfg80211_ops, sizeof(*ops), GFP_KERNEL); >> 6861 if (!ops) >> 6862 return NULL; >> 6863 >> 6864 ifp = netdev_priv(ndev); >> 6865 #ifdef CONFIG_PM >> >> (gdb) list *(wiphy_resume+0x591) >> 0xb751 is in wiphy_resume (net/wireless/sysfs.c:133). >> 128 int ret = 0; >> 129 >> 130 /* Age scan results with time spent in suspend */ >> 131 cfg80211_bss_age(rdev, get_seconds() - rdev->suspend_at); >> 132 >> 133 if (rdev->ops->resume) { >> 134 rtnl_lock(); >> 135 if (rdev->wiphy.registered) >> 136 ret = rdev_resume(rdev); >> 137 rtnl_unlock(); >> >> I'm unsure if this relates to the ordering of callbacks processed by >> dpm_run_callback. > > The problem is that our driver can not access the device as it has been > powered off during suspend. So in the resume we cleanup everything > calling wiphy_unregister() and wiphy_free(). This means the rdev in > wiphy_resume() above is already freed. Not sure how to handle this > properly. Probably we should do a proper rebind. > > Regards, > Arend > >> Thanks, >> Daniel >> >> -- [1] >> >> BUG: KASAN: use-after-free in wiphy_resume+0x591/0x5a0 [cfg80211] at >> addr ffff8803fefebb30 >> Read of size 8 by task kworker/u16:15/3066 >> CPU: 0 PID: 3066 Comm: kworker/u16:15 Not tainted 4.9.13-debug+ #7 >> Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 1.2.19 12/22/2016 >> Workqueue: events_unbound async_run_entry_fn >> ffff8803bffdf9d8 ffffffff880db6e1 ffff88042740ef00 ffff8803fefebb28 >> ffff8803bffdfa00 ffffffff87a4d941 ffff8803bffdfa98 ffff8803fefebb20 >> ffff88042740ef00 ffff8803bffdfa88 ffffffff87a4dbda ffff8803fb132360 >> Call Trace: >> [] dump_stack+0x85/0xc4 >> [] kasan_object_err+0x21/0x70 >> [] kasan_report_error+0x1fa/0x500 >> [] ? trace_hardirqs_on_caller+0x3fe/0x580 >> [] ? cfg80211_bss_age+0x9a/0xc0 [cfg80211] >> [] ? trace_hardirqs_on+0xd/0x10 >> [] ? wiphy_suspend+0xc70/0xc70 [cfg80211] >> [] __asan_report_load8_noabort+0x61/0x70 >> [] ? wiphy_suspend+0xbb0/0xc70 [cfg80211] >> [] ? wiphy_resume+0x591/0x5a0 [cfg80211] >> [] wiphy_resume+0x591/0x5a0 [cfg80211] >> [] ? wiphy_suspend+0xc70/0xc70 [cfg80211] >> [] dpm_run_callback+0x6e/0x4f0 >> [] device_resume+0x1c2/0x670 >> [] async_resume+0x1d/0x50 >> [] async_run_entry_fn+0xfe/0x610 >> [] process_one_work+0x716/0x1a50 >> [] ? process_one_work+0x679/0x1a50 >> [] ? _raw_spin_unlock_irq+0x3d/0x60 >> [] ? pwq_dec_nr_in_flight+0x2b0/0x2b0 >> [] worker_thread+0xe0/0x1460 >> [] ? process_one_work+0x1a50/0x1a50 >> [] kthread+0x222/0x2e0 >> [] ? kthread_park+0x80/0x80 >> [] ? kthread_park+0x80/0x80 >> [] ? kthread_park+0x80/0x80 >> [] ret_from_fork+0x2a/0x40 So the wiphy instance is unregistered *and* freed in our brcmf_pcie_resume() callback. However, the wiphy_resume() for this instance still comes right after that. I tried several fixes using API calls into driver core, but to no avail. So I came up with the fix below. Regards, Arend --- Sorry about the whitespace issues below (if any). My favorite mail client went haywire and I had to use gmail web interface and it looks like tabs are gone. --- diff --git a/net/wireless/core.c b/net/wireless/core.c index d888613..3af9c2e 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1096,6 +1096,17 @@ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev, } EXPORT_SYMBOL(cfg80211_stop_iface); +bool cfg80211_dev_in_list_rtnl(struct cfg80211_registered_device *rdev) +{ + struct cfg80211_registered_device *item; + + list_for_each_entry(item, &cfg80211_rdev_list, list) { + if (item == rdev) + return true; + } + return false; +} + static int cfg80211_netdev_notifier_call(struct notifier_block *nb, unsigned long state, void *ptr) { diff --git a/net/wireless/core.h b/net/wireless/core.h index e9afbc7..0407841 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -286,6 +286,7 @@ struct cfg80211_cqm_config { int cfg80211_dev_rename(struct cfg80211_registered_device *rdev, char *newname); +bool cfg80211_dev_in_list_rtnl(struct cfg80211_registered_device *rdev); void ieee80211_set_bitrate_flags(struct wiphy *wiphy); diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c index 16b6b59..e92459f 100644 --- a/net/wireless/sysfs.c +++ b/net/wireless/sysfs.c @@ -129,16 +129,19 @@ static int wiphy_resume(struct device *dev) struct cfg80211_registered_device *rdev = dev_to_rdev(dev); int ret = 0; + rtnl_lock(); + if (!cfg80211_dev_in_list_rtnl(rdev)) + goto done; + /* Age scan results with time spent in suspend */ cfg80211_bss_age(rdev, get_seconds() - rdev->suspend_at); if (rdev->ops->resume) { - rtnl_lock(); if (rdev->wiphy.registered) ret = rdev_resume(rdev); - rtnl_unlock(); } - +done: + rtnl_unlock(); return ret; }