netdev - Re: [PATCH v1 1/2] Bluetooth: coredump: Add hci_devcd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABBYNZKN+mHcvJkMB=1vvOyExF8_Tg2BnD-CemX3b14PoA1vkg@mail.gmail.com>
Date: Thu, 3 Jul 2025 09:42:24 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
Cc: marcel@...tmann.org, johan.hedberg@...il.com, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org, 
	netdev@...r.kernel.org, amitkumar.karwar@....com, sherry.sun@....com, 
	manjeet.gupta@....com
Subject: Re: [PATCH v1 1/2] Bluetooth: coredump: Add hci_devcd_unregister()
 for cleanup

Hi Neeraj,

On Thu, Jul 3, 2025 at 9:17 AM Neeraj Sanjay Kale
<neeraj.sanjaykale@....com> wrote:
>
> This adds hci_devcd_unregister() which can be called when driver is
> removed, which will cleanup the devcoredump data and cancel delayed
> dump_timeout work.
>
> With BTNXPUART driver, it is observed that after FW dump, if driver is
> removed and re-loaded, it creates hci1 interface instead of hci0
> interface.
>
> But after DEVCD_TIMEOUT (5 minutes) if driver is re-loaded, hci0 is
> created. This is because after FW dump, hci0 is not unregistered
> properly for DEVCD_TIMEOUT.
>
> With this patch, BTNXPUART is able to create hci0 after every FW dump
> and driver reload.
>
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
> ---
>  include/net/bluetooth/coredump.h | 3 +++
>  net/bluetooth/coredump.c         | 8 ++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h
> index 72f51b587a04..bc8856e4bfe7 100644
> --- a/include/net/bluetooth/coredump.h
> +++ b/include/net/bluetooth/coredump.h
> @@ -66,6 +66,7 @@ void hci_devcd_timeout(struct work_struct *work);
>
>  int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
>                        dmp_hdr_t dmp_hdr, notify_change_t notify_change);
> +void hci_devcd_unregister(struct hci_dev *hdev);
>  int hci_devcd_init(struct hci_dev *hdev, u32 dump_size);
>  int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb);
>  int hci_devcd_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len);
> @@ -85,6 +86,8 @@ static inline int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
>         return -EOPNOTSUPP;
>  }
>
> +static inline void hci_devcd_unregister(struct hci_dev *hdev) {}
> +
>  static inline int hci_devcd_init(struct hci_dev *hdev, u32 dump_size)
>  {
>         return -EOPNOTSUPP;
> diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
> index 819eacb38762..dd7bd40e3eba 100644
> --- a/net/bluetooth/coredump.c
> +++ b/net/bluetooth/coredump.c
> @@ -442,6 +442,14 @@ int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
>  }
>  EXPORT_SYMBOL(hci_devcd_register);
>
> +void hci_devcd_unregister(struct hci_dev *hdev)
> +{
> +       cancel_delayed_work(&hdev->dump.dump_timeout);
> +       skb_queue_purge(&hdev->dump.dump_q);
> +       dev_coredump_put(&hdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(hci_devcd_unregister);

The fact that the dump lives inside hdev is sort of the source of
these problems, specially if the dumps are not HCI traffic it might be
better off having the driver control its lifetime and not use
hdev->workqueue to schedule it.

>  static inline bool hci_devcd_enabled(struct hci_dev *hdev)
>  {
>         return hdev->dump.supported;
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz