* [PATCH] net: nfc: Fix use-after-free in local_cleanup()
@ 2023-01-04 12:57 Jisoo Jang
2023-01-05 11:41 ` Paolo Abeni
0 siblings, 1 reply; 2+ messages in thread
From: Jisoo Jang @ 2023-01-04 12:57 UTC (permalink / raw)
To: krzysztof.kozlowski, netdev
Cc: edumazet, kuba, pabeni, dokyungs, linuxlovemin
Fix a use-after-free that occurs in kfree_skb() called from
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.
Ensure kfree_skb() is called once when unregistering the device.
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
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@yonsei.ac.kr>
---
net/nfc/llcp_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 3364caabef8b..cbf2ef0af57b 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -156,7 +156,6 @@ static void local_cleanup(struct nfc_llcp_local *local)
cancel_work_sync(&local->tx_work);
cancel_work_sync(&local->rx_work);
cancel_work_sync(&local->timeout_work);
- kfree_skb(local->rx_pending);
del_timer_sync(&local->sdreq_timer);
cancel_work_sync(&local->sdreq_timeout_work);
nfc_llcp_free_sdp_tlv_list(&local->pending_sdreqs);
@@ -170,6 +169,7 @@ static void local_release(struct kref *ref)
list_del(&local->list);
local_cleanup(local);
+ kfree_skb(local->rx_pending);
kfree(local);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] net: nfc: Fix use-after-free in local_cleanup()
2023-01-04 12:57 [PATCH] net: nfc: Fix use-after-free in local_cleanup() Jisoo Jang
@ 2023-01-05 11:41 ` Paolo Abeni
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Abeni @ 2023-01-05 11:41 UTC (permalink / raw)
To: Jisoo Jang, krzysztof.kozlowski, netdev
Cc: edumazet, kuba, dokyungs, linuxlovemin
On Wed, 2023-01-04 at 21:57 +0900, Jisoo Jang wrote:
> Fix a use-after-free that occurs in kfree_skb() called from
> 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.
>
> Ensure kfree_skb() is called once when unregistering the device.
>
> 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
> 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@yonsei.ac.kr>
> ---
> net/nfc/llcp_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
> index 3364caabef8b..cbf2ef0af57b 100644
> --- a/net/nfc/llcp_core.c
> +++ b/net/nfc/llcp_core.c
> @@ -156,7 +156,6 @@ static void local_cleanup(struct nfc_llcp_local *local)
> cancel_work_sync(&local->tx_work);
> cancel_work_sync(&local->rx_work);
> cancel_work_sync(&local->timeout_work);
> - kfree_skb(local->rx_pending);
> del_timer_sync(&local->sdreq_timer);
> cancel_work_sync(&local->sdreq_timeout_work);
> nfc_llcp_free_sdp_tlv_list(&local->pending_sdreqs);
> @@ -170,6 +169,7 @@ static void local_release(struct kref *ref)
>
> list_del(&local->list);
> local_cleanup(local);
> + kfree_skb(local->rx_pending);
> kfree(local);
> }
>
With this change, local->tx_work/nfc_llcp_tx_work can be invoked after
nfc_unregister_device(). nfc_llcp_tx_work assumnes the nfc device is
alive, so I guess we need (part of) local_cleanup() being in place at
nfc de-registration time.
What about simply setting local->rx_pending to NULL in local_cleanup()?
Thanks!
Paolo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-05 11:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 12:57 [PATCH] net: nfc: Fix use-after-free in local_cleanup() Jisoo Jang
2023-01-05 11:41 ` Paolo Abeni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.