Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp1775ybl; Tue, 28 Jan 2020 17:09:31 -0800 (PST) X-Google-Smtp-Source: APXvYqwyy2/Fn5i7BwADyKJIMHt4hUuOpmhkO92FZv3ttsYzwFUZkhVC6d4kaJJFje8VKy36uReU X-Received: by 2002:aca:a857:: with SMTP id r84mr4672707oie.41.1580260170925; Tue, 28 Jan 2020 17:09:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1580260170; cv=none; d=google.com; s=arc-20160816; b=FA0MtsCyQoHu4t08icjZHunzFCb0r716znkLeYD8O1JLVmaye9CJF5bW9qYvT8aVxf rg+1nMDUXjSPKm8g+Bq1T4TzRmMqCiT/I6reYUGq0QGkfmMPOPe0amIiTvN+pt0szSam qdVoIt5hN3oOG2oZv4zyJnp5PdpdPnSMy7fmN8aZgjJEDAj5Xkr/v7TQAbnXHHPix3jI ozGfNLp8RZSNUaA3e2HipkheT6lVBylIVN8bD1t77/+e58+J6ni4Qbk3/NmgssNODVwF 7+P2VRr45ZNtFMZZpRHk++hA+bX5qzdyXi8uWbqsG7XlJxrT23z92CiZM03FWV9mRahK Aw4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=hfPnwCqzCAhNK/qRWkWU3pxM5PERG8Ehj+A9uu9jn7s=; b=MDqn67ATo7e64dV4N37CNCPK699XrYMMamBFCMNNilYxV8ivQhTWPgOxzccbmOB6YJ 04O6wnDZmjBBHX8wNAQ8USmazYnsK+BFMktOdViHaj2foPKjypcGehwq4DH5Gztqj4MY NAuAGKhjeWbnKv0mYkpf7FIxe0phelY2d4LTcLg6OfFAICz8/1o/CtL8KOPAXEGngIVE W5Dcs1Ws8JBiYjLjczqMnk2RjK8LmQSojZP2L/zjuS+BdJK9grq3RO5jm1cWdSbcjSQ6 PyzvRUNk0+tqWZRDApqyb5xvkP600QjSZP5asdzj1DokhDuY12ru+nVkAa5gf/GwiORa ou7A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@broadcom.com header.s=google header.b=PV5nGiRe; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=broadcom.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j2si272917otk.164.2020.01.28.17.08.38; Tue, 28 Jan 2020 17:09:30 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@broadcom.com header.s=google header.b=PV5nGiRe; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=broadcom.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726758AbgA2A60 (ORCPT + 99 others); Tue, 28 Jan 2020 19:58:26 -0500 Received: from mail-io1-f66.google.com ([209.85.166.66]:35256 "EHLO mail-io1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726391AbgA2A6Z (ORCPT ); Tue, 28 Jan 2020 19:58:25 -0500 Received: by mail-io1-f66.google.com with SMTP id h8so16670372iob.2 for ; Tue, 28 Jan 2020 16:58:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hfPnwCqzCAhNK/qRWkWU3pxM5PERG8Ehj+A9uu9jn7s=; b=PV5nGiReMA0sAjLADHB6cjIFeP0lOuZ2izL3OeGoiIvgFCUiygVFxPpzwLVE5KY48/ fNV7VLkyKdLRhdw6lXTdtjxDSp1Kfm98KBAR0mc8RBQRe6pToRC6dObO8PA0tI98u+En ahQO0LZozEg2pDBtBjOaZHpxcaSgzRJcywL2E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hfPnwCqzCAhNK/qRWkWU3pxM5PERG8Ehj+A9uu9jn7s=; b=CB8dVWlu0fEhi9iCDnlZYSX8Kgp727DEYXDA5a1WiYawlNGTq7k8mWEpDgy8k7QKPq AALfjl2JvM1oqaF75wx7WpUZ4fWdupjELYkh+Orf+2N6YnyjBvgzWLLMmz+c8XSnsVqk XnpzqlSx5HLBrWEBlgKhn6BzOx5NEGM1tjzeHq8b/mddvaE8uQoT3aJodIZtYMvVL7nf z9B7UrklTbMI/ObnB/k9ANh1RXiimNlXm/FTAVxmgex1QUPPl5m7enFu0rKeoPrDIG9M l45mS7JhAkLBqQ8EBDoA6kuD4WZJjl1gKdaJjJsu/VEupDm3HdFAVJP8kk7oJ0Izxug/ m1Bw== X-Gm-Message-State: APjAAAXNhRXB8D38paxYYSIsDoFJgRRsJBoGHgGkY6pJtLKffDUjfrhi 6j+DTomK3Jh5we+PZLweg0hk138E4vmXdieJMDb1mw== X-Received: by 2002:a05:6602:22cd:: with SMTP id e13mr15272247ioe.251.1580259505025; Tue, 28 Jan 2020 16:58:25 -0800 (PST) MIME-Version: 1.0 References: <20200128221457.12467-1-linux@roeck-us.net> <20200129000551.GA17256@roeck-us.net> In-Reply-To: <20200129000551.GA17256@roeck-us.net> From: Franky Lin Date: Tue, 28 Jan 2020 16:57:59 -0800 Message-ID: Subject: Re: [PATCH] brcmfmac: abort and release host after error To: Guenter Roeck Cc: Doug Anderson , Kalle Valo , Arend van Spriel , Hante Meuleman , Chi-Hsien Lin , Dan Carpenter , linux-wireless , "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER" , brcm80211-dev-list , netdev , LKML , Matthias Kaehlcke , Brian Norris Content-Type: text/plain; charset="UTF-8" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Tue, Jan 28, 2020 at 4:05 PM Guenter Roeck wrote: > > On Tue, Jan 28, 2020 at 03:14:45PM -0800, Doug Anderson wrote: > > Hi, > > > > On Tue, Jan 28, 2020 at 2:15 PM Guenter Roeck wrote: > > > > > > With commit 216b44000ada ("brcmfmac: Fix use after free in > > > brcmf_sdio_readframes()") applied, we see locking timeouts in > > > brcmf_sdio_watchdog_thread(). > > > > > > brcmfmac: brcmf_escan_timeout: timer expired > > > INFO: task brcmf_wdog/mmc1:621 blocked for more than 120 seconds. > > > Not tainted 4.19.94-07984-g24ff99a0f713 #1 > > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > > > brcmf_wdog/mmc1 D 0 621 2 0x00000000 last_sleep: 2440793077. last_runnable: 2440766827 > > > [] (__schedule) from [] (schedule+0x98/0xc4) > > > [] (schedule) from [] (__mmc_claim_host+0x154/0x274) > > > [] (__mmc_claim_host) from [] (brcmf_sdio_watchdog_thread+0x1b0/0x1f8 [brcmfmac]) > > > [] (brcmf_sdio_watchdog_thread [brcmfmac]) from [] (kthread+0x178/0x180) > > > > > > In addition to restarting or exiting the loop, it is also necessary to > > > abort the command and to release the host. > > > > > > Fixes: 216b44000ada ("brcmfmac: Fix use after free in brcmf_sdio_readframes()") > > > Cc: Dan Carpenter > > > Cc: Matthias Kaehlcke > > > Cc: Brian Norris > > > Cc: Douglas Anderson > > > Signed-off-by: Guenter Roeck > > > --- > > > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > > > index f9df95bc7fa1..2e1c23c7269d 100644 > > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > > > @@ -1938,6 +1938,8 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes) > > > if (brcmf_sdio_hdparse(bus, bus->rxhdr, &rd_new, > > > BRCMF_SDIO_FT_NORMAL)) { > > > rd->len = 0; > > > + brcmf_sdio_rxfail(bus, true, true); > > > + sdio_release_host(bus->sdiodev->func1); > > > > I don't know much about this driver so I don't personally know if > > "true, true" is the correct thing to pass to brcmf_sdio_rxfail(), but > > it seems plausible. Definitely the fix to call sdio_release_host() is > > sane. > > > > Thus, unless someone knows for sure that brcmf_sdio_rxfail()'s > > parameters should be different: > > > Actually, looking at brcmf_sdio_hdparse() and its other callers, > I think it may not be needed at all - other callers don't do it, and > there already are some calls to brcmf_sdio_rxfail() in that function. > It would be nice though to get a confirmation before I submit v2. I think invoking rxfail with both abort and NACK set to true is the right thing to do here so that the pipeline can be properly purged. Thanks! Acked-by: franky.lin@broadcom.com