netdev - Re: [PATCH 1/1] Bluetooth: fix race condition in hci_cmd_sync

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>] [day] [month] [year] [list]

Date:   Fri, 3 Mar 2023 10:09:34 -0800
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     lm0963 <lm0963hack@...il.com>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>, davem@...emloft.net,
        edumazet@...gle.com, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, jkosina@...e.cz,
        hdegoede@...hat.com, david.rheinsberg@...il.com,
        wsa+renesas@...g-engineering.com, linux@...ssschuh.net,
        linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] Bluetooth: fix race condition in hci_cmd_sync_clear

Hi Min,

On Wed, Mar 1, 2023 at 11:21 PM lm0963 <lm0963hack@...il.com> wrote:
>
> There is a potential race condition in hci_cmd_sync_work and
> hci_cmd_sync_clear, and could lead to use-after-free. For instance,
> hci_cmd_sync_work is added to the 'req_workqueue' after cancel_work_sync
> The entry of 'cmd_sync_work_list' may be freed in hci_cmd_sync_clear, and
> causing kernel panic when it is used in hci_cmd_sync_work.
>
> Here's the call trace:
>
> dump_stack_lvl+0x49/0x63
> print_report.cold+0x5e/0x5d3
> ? hci_cmd_sync_work+0x282/0x320
> kasan_report+0xaa/0x120
> ? hci_cmd_sync_work+0x282/0x320
> __asan_report_load8_noabort+0x14/0x20
> hci_cmd_sync_work+0x282/0x320
> process_one_work+0x77b/0x11c0
> ? _raw_spin_lock_irq+0x8e/0xf0
> worker_thread+0x544/0x1180
> ? poll_idle+0x1e0/0x1e0
> kthread+0x285/0x320
> ? process_one_work+0x11c0/0x11c0
> ? kthread_complete_and_exit+0x30/0x30
> ret_from_fork+0x22/0x30
> </TASK>
>
> Allocated by task 266:
> kasan_save_stack+0x26/0x50
> __kasan_kmalloc+0xae/0xe0
> kmem_cache_alloc_trace+0x191/0x350
> hci_cmd_sync_queue+0x97/0x2b0
> hci_update_passive_scan+0x176/0x1d0
> le_conn_complete_evt+0x1b5/0x1a00
> hci_le_conn_complete_evt+0x234/0x340
> hci_le_meta_evt+0x231/0x4e0
> hci_event_packet+0x4c5/0xf00
> hci_rx_work+0x37d/0x880
> process_one_work+0x77b/0x11c0
> worker_thread+0x544/0x1180
> kthread+0x285/0x320
> ret_from_fork+0x22/0x30
>
> Freed by task 269:
> kasan_save_stack+0x26/0x50
> kasan_set_track+0x25/0x40
> kasan_set_free_info+0x24/0x40
> ____kasan_slab_free+0x176/0x1c0
> __kasan_slab_free+0x12/0x20
> slab_free_freelist_hook+0x95/0x1a0
> kfree+0xba/0x2f0
> hci_cmd_sync_clear+0x14c/0x210
> hci_unregister_dev+0xff/0x440
> vhci_release+0x7b/0xf0
> __fput+0x1f3/0x970
> ____fput+0xe/0x20
> task_work_run+0xd4/0x160
> do_exit+0x8b0/0x22a0
> do_group_exit+0xba/0x2a0
> get_signal+0x1e4a/0x25b0
> arch_do_signal_or_restart+0x93/0x1f80
> exit_to_user_mode_prepare+0xf5/0x1a0
> syscall_exit_to_user_mode+0x26/0x50
> ret_from_fork+0x15/0x30
>
> Signed-off-by: Min Li <lm0963hack@...il.com>
> ---
>  net/bluetooth/hci_sync.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 117eedb6f709..3103daf49d63 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -643,6 +643,7 @@ void hci_cmd_sync_clear(struct hci_dev *hdev)
>   cancel_work_sync(&hdev->cmd_sync_work);
>   cancel_work_sync(&hdev->reenable_adv_work);
>
> + mutex_lock(&hdev->cmd_sync_work_lock);
>   list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) {
>   if (entry->destroy)
>   entry->destroy(hdev, entry->data, -ECANCELED);
> @@ -650,6 +651,7 @@ void hci_cmd_sync_clear(struct hci_dev *hdev)
>   list_del(&entry->list);
>   kfree(entry);
>   }
> + mutex_unlock(&hdev->cmd_sync_work_lock);
>  }

The code style of this one seems broken, did you generate it using git
format-patch?

>  void __hci_cmd_sync_cancel(struct hci_dev *hdev, int err)
> --
> 2.25.1



-- 
Luiz Augusto von Dentz