[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b2232e8d-275b-4e98-84fe-bbb33e3c6b7e@gmail.com>
Date: Sun, 28 Jul 2024 03:39:18 +0900
From: Yunseong Kim <yskelg@...il.com>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>,
Luiz Augusto von Dentz <luiz.dentz@...il.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
stable@...r.kernel.org, linux-kernel@...r.kernel.org,
Yeoreum Yun <yeoreum.yun@....com>
Subject: Re: [PATCH v2] Bluetooth: hci_core: fix suspicious RCU usage in
hci_conn_drop()
Hi Tetsuo,
> Excuse me, but I can't interpret why this patch solves the warning.
>
> The warning says that list_for_each_entry_rcu() { } in
> ieee80211_check_combinations() is called outside of rcu_read_lock() and
> rcu_read_unlock() pair, doesn't it? How does that connected to
> guarding hci_dev_test_flag() and queue_delayed_work() with rcu_read_lock()
> and rcu_read_unlock() pair? Unless you guard list_for_each_entry_rcu() { }
> in ieee80211_check_combinations() with rcu_read_lock() and rcu_read_unlock()
> pair (or annotate that appropriate locks are already held), I can't expect
> that the warning will be solved...
Thank you for the code review.
Sorry, I apologize for attaching the wrong kernel dump.
> Also, what guarantees that drain_workqueue() won't be disturbed by
> queue_work(disc_work) which will be called after "timeo" delay, for you are
> not explicitly cancelling scheduled "disc_work" (unlike "cmd_timer" work
> and "ncmd_timer" work shown below) before calling drain_workqueue() ?
>
> /* Cancel these to avoid queueing non-chained pending work */
> hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
> /* Wait for
> *
> * if (!hci_dev_test_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE))
> * queue_delayed_work(&hdev->{cmd,ncmd}_timer)
> *
> * inside RCU section to see the flag or complete scheduling.
> */
> synchronize_rcu();
> /* Explicitly cancel works in case scheduled after setting the flag. */
> cancel_delayed_work(&hdev->cmd_timer);
> cancel_delayed_work(&hdev->ncmd_timer);
>
> /* Avoid potential lockdep warnings from the *_flush() calls by
> * ensuring the workqueue is empty up front.
> */
> drain_workqueue(hdev->workqueue);
Please bear with me for a moment.
I'll attach the correct kernel dump and resend the patch email.
Warm regards,
Yunseong Kim
Powered by blists - more mailing lists