| 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
| ||
|
Date: Mon, 18 Apr 2022 06:41:33 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Lin Ma <linma@....edu.cn>
Cc: krzk@...nel.org, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, mudongliangabcd@...il.com
Subject: Re: [PATCH v0] nfc: nci: add flush_workqueue to prevent uaf
On Wed, Apr 13, 2022 at 12:04:30AM +0800, Lin Ma wrote:
> Our detector found a concurrent use-after-free bug when detaching an
> NCI device. The main reason for this bug is the unexpected scheduling
> between the used delayed mechanism (timer and workqueue).
>
> The race can be demonstrated below:
>
> Thread-1 Thread-2
> | nci_dev_up()
> | nci_open_device()
> | __nci_request(nci_reset_req)
> | nci_send_cmd
> | queue_work(cmd_work)
> nci_unregister_device() |
> nci_close_device() | ...
> del_timer_sync(cmd_timer)[1] |
> ... | Worker
> nci_free_device() | nci_cmd_work()
> kfree(ndev)[3] | mod_timer(cmd_timer)[2]
>
> In short, the cleanup routine thought that the cmd_timer has already
> been detached by [1] but the mod_timer can re-attach the timer [2], even
> it is already released [3], resulting in UAF.
>
> This UAF is easy to trigger, crash trace by POC is like below
>
> [ 66.703713] ==================================================================
> [ 66.703974] BUG: KASAN: use-after-free in enqueue_timer+0x448/0x490
> [ 66.703974] Write of size 8 at addr ffff888009fb7058 by task kworker/u4:1/33
> [ 66.703974]
> [ 66.703974] CPU: 1 PID: 33 Comm: kworker/u4:1 Not tainted 5.18.0-rc2 #5
> [ 66.703974] Workqueue: nfc2_nci_cmd_wq nci_cmd_work
> [ 66.703974] Call Trace:
> [ 66.703974] <TASK>
> [ 66.703974] dump_stack_lvl+0x57/0x7d
> [ 66.703974] print_report.cold+0x5e/0x5db
> [ 66.703974] ? enqueue_timer+0x448/0x490
> [ 66.703974] kasan_report+0xbe/0x1c0
> [ 66.703974] ? enqueue_timer+0x448/0x490
> [ 66.703974] enqueue_timer+0x448/0x490
> [ 66.703974] __mod_timer+0x5e6/0xb80
> [ 66.703974] ? mark_held_locks+0x9e/0xe0
> [ 66.703974] ? try_to_del_timer_sync+0xf0/0xf0
> [ 66.703974] ? lockdep_hardirqs_on_prepare+0x17b/0x410
> [ 66.703974] ? queue_work_on+0x61/0x80
> [ 66.703974] ? lockdep_hardirqs_on+0xbf/0x130
> [ 66.703974] process_one_work+0x8bb/0x1510
> [ 66.703974] ? lockdep_hardirqs_on_prepare+0x410/0x410
> [ 66.703974] ? pwq_dec_nr_in_flight+0x230/0x230
> [ 66.703974] ? rwlock_bug.part.0+0x90/0x90
> [ 66.703974] ? _raw_spin_lock_irq+0x41/0x50
> [ 66.703974] worker_thread+0x575/0x1190
> [ 66.703974] ? process_one_work+0x1510/0x1510
> [ 66.703974] kthread+0x2a0/0x340
> [ 66.703974] ? kthread_complete_and_exit+0x20/0x20
> [ 66.703974] ret_from_fork+0x22/0x30
> [ 66.703974] </TASK>
> [ 66.703974]
> [ 66.703974] Allocated by task 267:
> [ 66.703974] kasan_save_stack+0x1e/0x40
> [ 66.703974] __kasan_kmalloc+0x81/0xa0
> [ 66.703974] nci_allocate_device+0xd3/0x390
> [ 66.703974] nfcmrvl_nci_register_dev+0x183/0x2c0
> [ 66.703974] nfcmrvl_nci_uart_open+0xf2/0x1dd
> [ 66.703974] nci_uart_tty_ioctl+0x2c3/0x4a0
> [ 66.703974] tty_ioctl+0x764/0x1310
> [ 66.703974] __x64_sys_ioctl+0x122/0x190
> [ 66.703974] do_syscall_64+0x3b/0x90
> [ 66.703974] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 66.703974]
> [ 66.703974] Freed by task 406:
> [ 66.703974] kasan_save_stack+0x1e/0x40
> [ 66.703974] kasan_set_track+0x21/0x30
> [ 66.703974] kasan_set_free_info+0x20/0x30
> [ 66.703974] __kasan_slab_free+0x108/0x170
> [ 66.703974] kfree+0xb0/0x330
> [ 66.703974] nfcmrvl_nci_unregister_dev+0x90/0xd0
> [ 66.703974] nci_uart_tty_close+0xdf/0x180
> [ 66.703974] tty_ldisc_kill+0x73/0x110
> [ 66.703974] tty_ldisc_hangup+0x281/0x5b0
> [ 66.703974] __tty_hangup.part.0+0x431/0x890
> [ 66.703974] tty_release+0x3a8/0xc80
> [ 66.703974] __fput+0x1f0/0x8c0
> [ 66.703974] task_work_run+0xc9/0x170
> [ 66.703974] exit_to_user_mode_prepare+0x194/0x1a0
> [ 66.703974] syscall_exit_to_user_mode+0x19/0x50
> [ 66.703974] do_syscall_64+0x48/0x90
> [ 66.703974] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> To fix the UAF, this patch adds flush_workqueue() to ensure the
> nci_cmd_work is finished before the following del_timer_sync.
> This combination will promise the timer is actually detached.
>
> Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
> Signed-off-by: Lin Ma <linma@....edu.cn>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
> net/nfc/nci/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
> index d2537383a3e8..0d7763c322b5 100644
> --- a/net/nfc/nci/core.c
> +++ b/net/nfc/nci/core.c
> @@ -560,6 +560,10 @@ static int nci_close_device(struct nci_dev *ndev)
> mutex_lock(&ndev->req_lock);
>
> if (!test_and_clear_bit(NCI_UP, &ndev->flags)) {
> + /* Need to flush the cmd wq in case
> + * there is a queued/running cmd_work
> + */
> + flush_workqueue(ndev->cmd_wq);
> del_timer_sync(&ndev->cmd_timer);
I have been wondering about this and the same code further below.
What prevents the command timer from firing after the call to
flush_workqueue() ?
Thanks,
Guenter
> del_timer_sync(&ndev->data_timer);
> mutex_unlock(&ndev->req_lock);
Powered by blists - more mailing lists