netdev - Re: [PATCH v2] net: nfc: Fix use-after-free in local

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

Message-ID: <Y7nBqiZkBt2P6hyF@x130>
Date:   Sat, 7 Jan 2023 11:02:02 -0800
From:   Saeed Mahameed <saeed@...nel.org>
To:     Jisoo Jang <jisoo.jang@...sei.ac.kr>
Cc:     pabeni@...hat.com, krzysztof.kozlowski@...aro.org,
        netdev@...r.kernel.org, edumazet@...gle.com, kuba@...nel.org,
        dokyungs@...sei.ac.kr, linuxlovemin@...sei.ac.kr
Subject: Re: [PATCH v2] net: nfc: Fix use-after-free in local_cleanup()

On 06 Jan 14:50, Jisoo Jang wrote:
>local_cleanup(). When detaching an nfc device, local_cleanup()
>called from nfc_llcp_unregister_device() frees local->rx_pending
>and cancels local->rx_work. So the socket allocated before
>unregister is not set null by nfc_llcp_rx_work().
>local_cleanup() called from local_release() frees local->rx_pending
>again, which leads to the bug.
>
>Set local->rx_pending to NULL in local_cleanup()
>
>Found by a modified version of syzkaller.
>
>BUG: KASAN: use-after-free in kfree_skb
>Call Trace:
> kfree_skb
> local_cleanup
> nfc_llcp_local_put
> llcp_sock_destruct
> __sk_destruct
> sk_destruct
> __sk_free
> sk_free
> llcp_sock_release
> __sock_release
> sock_close
> __fput
> task_work_run
> exit_to_user_mode_prepare
> syscall_exit_to_user_mode
> do_syscall_64
> entry_SYSCALL_64_after_hwframe
>
>Allocate by:
> __alloc_skb
> pn533_recv_response
> __usb_hcd_giveback_urb
> usb_hcd_giveback_urb
> dummy_timer
> call_timer_fn
> run_timer_softirq
> __do_softirq
>
>Freed by:
> kfree_skbmem
> kfree_skb
> local_cleanup

Why local_cleanup is called explicitly here ? 
The next thing nfc_llcp_unregister_device() will call is
nfc_llcp_local_put()->kref_put->local_release()->local_cleanup()


> nfc_llcp_unregister_device
> nfc_unregister_device
> pn53x_unregister_nfc
> pn533_usb_disconnect
> usb_unbind_interface
> device_release_driver_internal
> bus_remove_device
> device_del
> usb_disable_device
> usb_disconnect
> hub_event
> process_one_work
> worker_thread
> kthread
> ret_from_fork
>
>
>Signed-off-by: Jisoo Jang <jisoo.jang@...sei.ac.kr>
>---
>v1->v2: set local->rx_pending to NULL instead move kfree_skb()
>
> net/nfc/llcp_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
>index 3364caabef8b..a27e1842b2a0 100644
>--- a/net/nfc/llcp_core.c
>+++ b/net/nfc/llcp_core.c
>@@ -157,6 +157,7 @@ static void local_cleanup(struct nfc_llcp_local *local)
> 	cancel_work_sync(&local->rx_work);
> 	cancel_work_sync(&local->timeout_work);
> 	kfree_skb(local->rx_pending);
>+	local->rx_pending = NULL;
> 	del_timer_sync(&local->sdreq_timer);
> 	cancel_work_sync(&local->sdreq_timeout_work);
> 	nfc_llcp_free_sdp_tlv_list(&local->pending_sdreqs);
>-- 
>2.25.1
>