diff mbox series

[v1] Bluetooth: af_bluetooth: Fix deadlock

Message ID 20240301185034.2756103-1-luiz.dentz@gmail.com
State New
Headers show
Series [v1] Bluetooth: af_bluetooth: Fix deadlock | expand

Commit Message

Luiz Augusto von Dentz March 1, 2024, 6:50 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Attemting to do sock_lock on .recvmsg may cause a deadlock as shown
bellow, so instead of using sock_sock this uses sk_receive_queue.lock
on bt_sock_ioctl to avoid the UAF:

INFO: task kworker/u9:1:121 blocked for more than 30 seconds.
      Not tainted 6.7.6-lemon #183
Workqueue: hci0 hci_rx_work
Call Trace:
 <TASK>
 __schedule+0x37d/0xa00
 schedule+0x32/0xe0
 __lock_sock+0x68/0xa0
 ? __pfx_autoremove_wake_function+0x10/0x10
 lock_sock_nested+0x43/0x50
 l2cap_sock_recv_cb+0x21/0xa0
 l2cap_recv_frame+0x55b/0x30a0
 ? psi_task_switch+0xeb/0x270
 ? finish_task_switch.isra.0+0x93/0x2a0
 hci_rx_work+0x33a/0x3f0
 process_one_work+0x13a/0x2f0
 worker_thread+0x2f0/0x410
 ? __pfx_worker_thread+0x10/0x10
 kthread+0xe0/0x110
 ? __pfx_kthread+0x10/0x10
 ret_from_fork+0x2c/0x50
 ? __pfx_kthread+0x10/0x10
 ret_from_fork_asm+0x1b/0x30
 </TASK>

Fixes: 2e07e8348ea4 ("Bluetooth: af_bluetooth: Fix Use-After-Free in bt_sock_recvmsg")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/af_bluetooth.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

patchwork-bot+bluetooth@kernel.org March 4, 2024, 4:30 p.m. UTC | #1 
Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri,  1 Mar 2024 13:50:34 -0500 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> Attemting to do sock_lock on .recvmsg may cause a deadlock as shown
> bellow, so instead of using sock_sock this uses sk_receive_queue.lock
> on bt_sock_ioctl to avoid the UAF:
> 
> INFO: task kworker/u9:1:121 blocked for more than 30 seconds.
>       Not tainted 6.7.6-lemon #183
> Workqueue: hci0 hci_rx_work
> Call Trace:
>  <TASK>
>  __schedule+0x37d/0xa00
>  schedule+0x32/0xe0
>  __lock_sock+0x68/0xa0
>  ? __pfx_autoremove_wake_function+0x10/0x10
>  lock_sock_nested+0x43/0x50
>  l2cap_sock_recv_cb+0x21/0xa0
>  l2cap_recv_frame+0x55b/0x30a0
>  ? psi_task_switch+0xeb/0x270
>  ? finish_task_switch.isra.0+0x93/0x2a0
>  hci_rx_work+0x33a/0x3f0
>  process_one_work+0x13a/0x2f0
>  worker_thread+0x2f0/0x410
>  ? __pfx_worker_thread+0x10/0x10
>  kthread+0xe0/0x110
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork+0x2c/0x50
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork_asm+0x1b/0x30
>  </TASK>
> 
> [...]

Here is the summary with links:
  - [v1] Bluetooth: af_bluetooth: Fix deadlock
    https://git.kernel.org/bluetooth/bluetooth-next/c/40245851528a

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index b93464ac3517..67604ccec2f4 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -309,14 +309,11 @@  int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	if (flags & MSG_OOB)
 		return -EOPNOTSUPP;
 
-	lock_sock(sk);
-
 	skb = skb_recv_datagram(sk, flags, &err);
 	if (!skb) {
 		if (sk->sk_shutdown & RCV_SHUTDOWN)
 			err = 0;
 
-		release_sock(sk);
 		return err;
 	}
 
@@ -346,8 +343,6 @@  int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 
 	skb_free_datagram(sk, skb);
 
-	release_sock(sk);
-
 	if (flags & MSG_TRUNC)
 		copied = skblen;
 
@@ -570,10 +565,11 @@  int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		if (sk->sk_state == BT_LISTEN)
 			return -EINVAL;
 
-		lock_sock(sk);
+		spin_lock(&sk->sk_receive_queue.lock);
 		skb = skb_peek(&sk->sk_receive_queue);
 		amount = skb ? skb->len : 0;
-		release_sock(sk);
+		spin_unlock(&sk->sk_receive_queue.lock);
+
 		err = put_user(amount, (int __user *)arg);
 		break;