From patchwork Wed Nov 23 20:18:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 628444 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 01C5FC4332F for ; Wed, 23 Nov 2022 20:22:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239724AbiKWUWf (ORCPT ); Wed, 23 Nov 2022 15:22:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41820 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239361AbiKWUV0 (ORCPT ); Wed, 23 Nov 2022 15:21:26 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C2E80C769B; Wed, 23 Nov 2022 12:19:13 -0800 (PST) Message-ID: <20221123201625.435907114@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1669234737; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=N5rjif6fyxnP/Qgbwg5ZMeD/njPbRjLjFEtdr1g8NrQ=; b=R9QHOKD307W6xOOboyxsH4Y9NpQA+cS0rm9nYLPCkqLpbXXlf7QKZk/un/aTDPSM8r8ke0 WHn6izB5oy4rGA83r+5+1TwRlumiU+VVFCU8PQK8GrvvfAcNBSA2v5ocQMfB0A6kU6r3JH qEpMYrGt9z+IPS6/Ctb6ddMTtTTo4VoAS74Ct/ec6emH6p+fQ1SzrtEc9vBL6znzRNP713 FpXmcKZlEU0TzqQmwK+wUAzgNEB/g7Cib+bRz2y6AMYGMbjRPVXh1r6MxInXm4e1qCGRgi CpDwasqC/RCHLMlEbhVfXMvArunDJPBguejkD+XStLfBV3N/TqEeBco5rIT2Bw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1669234737; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=N5rjif6fyxnP/Qgbwg5ZMeD/njPbRjLjFEtdr1g8NrQ=; b=2dJK9SFpNO7GBTAPIsBlJ30G65FIbTDXFF2qtscws/0M6SmS23TaGhsCxRNhPmvqmPY9+U LNq7gAjOreuCrOBg== From: Thomas Gleixner To: LKML Cc: Linus Torvalds , Steven Rostedt , Anna-Maria Behnsen , Peter Zijlstra , Stephen Boyd , Guenter Roeck , Andrew Morton , Julia Lawall , Arnd Bergmann , Viresh Kumar , Marc Zyngier , Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz , linux-bluetooth@vger.kernel.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org, Jacob Keller Subject: [patch V3 17/17] Bluetooth: hci_qca: Fix the teardown problem for real References: <20221123201306.823305113@linutronix.de> MIME-Version: 1.0 Date: Wed, 23 Nov 2022 21:18:57 +0100 (CET) Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org While discussing solutions for the teardown problem which results from circular dependencies between timers and workqueues, where timers schedule work from their timer callback and workqueues arm the timers from work items, it was discovered that the recent fix to the QCA code is incorrect. That commit fixes the obvious problem of using del_timer() instead of del_timer_sync() and reorders the teardown calls to destroy_workqueue(wq); del_timer_sync(t); This makes it less likely to explode, but it's still broken: destroy_workqueue(wq); /* After this point @wq cannot be touched anymore */ ---> timer expires queue_work(wq) <---- Results in a NULl pointer dereference deep in the work queue core code. del_timer_sync(t); Use the new timer_shutdown_sync() function to ensure that the timers are disarmed, no timer callbacks are running and the timers cannot be armed again. This restores the original teardown sequence: timer_shutdown_sync(t); destroy_workqueue(wq); which is now correct because the timer core silently ignores potential rearming attempts which can happen when destroy_workqueue() drains pending work before mopping up the workqueue. Fixes: 72ef98445aca ("Bluetooth: hci_qca: Use del_timer_sync() before freeing") Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Jacob Keller Acked-by: Luiz Augusto von Dentz Cc: Marcel Holtmann Cc: Johan Hedberg Cc: linux-bluetooth@vger.kernel.org Cc: "David S. Miller" Cc: Eric Dumazet Cc: Jakub Kicinski Cc: Paolo Abeni Cc: netdev@vger.kernel.org Link: https://lore.kernel.org/all/87iljhsftt.ffs@tglx --- drivers/bluetooth/hci_qca.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -696,9 +696,15 @@ static int qca_close(struct hci_uart *hu skb_queue_purge(&qca->tx_wait_q); skb_queue_purge(&qca->txq); skb_queue_purge(&qca->rx_memdump_q); + /* + * Shut the timers down so they can't be rearmed when + * destroy_workqueue() drains pending work which in turn might try + * to arm a timer. After shutdown rearm attempts are silently + * ignored by the timer core code. + */ + timer_shutdown_sync(&qca->tx_idle_timer); + timer_shutdown_sync(&qca->wake_retrans_timer); destroy_workqueue(qca->workqueue); - del_timer_sync(&qca->tx_idle_timer); - del_timer_sync(&qca->wake_retrans_timer); qca->hu = NULL; kfree_skb(qca->rx_skb);