2021-10-07 19:07:17

by Nguyen Dinh Phi

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_sock: purge socket queues in the destruct() callback

The receive path may take the socket right before hci_sock_release(),
but it may enqueue the packets to the socket queues after the call to
skb_queue_purge(), therefore the socket can be destroyed without clear
its queues completely.

Moving these skb_queue_purge() to the hci_sock_destruct() will fix this
issue, because nothing is referencing the socket at this point.

Signed-off-by: Nguyen Dinh Phi <[email protected]>
Reported-by: [email protected]
---
net/bluetooth/hci_sock.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index d0dad1fafe07..446573a12571 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -889,10 +889,6 @@ static int hci_sock_release(struct socket *sock)
}

sock_orphan(sk);
-
- skb_queue_purge(&sk->sk_receive_queue);
- skb_queue_purge(&sk->sk_write_queue);
-
release_sock(sk);
sock_put(sk);
return 0;
@@ -2058,6 +2054,12 @@ static int hci_sock_getsockopt(struct socket *sock, int level, int optname,
return err;
}

+static void hci_sock_destruct(struct sock *sk)
+{
+ skb_queue_purge(&sk->sk_receive_queue);
+ skb_queue_purge(&sk->sk_write_queue);
+}
+
static const struct proto_ops hci_sock_ops = {
.family = PF_BLUETOOTH,
.owner = THIS_MODULE,
@@ -2111,6 +2113,7 @@ static int hci_sock_create(struct net *net, struct socket *sock, int protocol,

sock->state = SS_UNCONNECTED;
sk->sk_state = BT_OPEN;
+ sk->sk_destruct = hci_sock_destruct;

bt_sock_link(&hci_sk_list, sk);
return 0;
--
2.25.1


2021-10-07 22:08:06

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: hci_sock: purge socket queues in the destruct() callback

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=559391

---Test result---

Test Summary:
CheckPatch PASS 0.75 seconds
GitLint PASS 0.32 seconds
BuildKernel PASS 709.73 seconds
TestRunner: Setup PASS 514.21 seconds
TestRunner: l2cap-tester PASS 11.22 seconds
TestRunner: bnep-tester PASS 5.71 seconds
TestRunner: mgmt-tester PASS 95.57 seconds
TestRunner: rfcomm-tester PASS 7.51 seconds
TestRunner: sco-tester PASS 7.62 seconds
TestRunner: smp-tester PASS 7.42 seconds
TestRunner: userchan-tester PASS 6.19 seconds



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (43.32 kB)
bnep-tester.log (3.48 kB)
mgmt-tester.log (622.86 kB)
rfcomm-tester.log (11.41 kB)
sco-tester.log (13.60 kB)
smp-tester.log (11.55 kB)
userchan-tester.log (6.22 kB)
Download all attachments

2021-10-12 15:40:21

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_sock: purge socket queues in the destruct() callback

Hi Nguyen,

> The receive path may take the socket right before hci_sock_release(),
> but it may enqueue the packets to the socket queues after the call to
> skb_queue_purge(), therefore the socket can be destroyed without clear
> its queues completely.
>
> Moving these skb_queue_purge() to the hci_sock_destruct() will fix this
> issue, because nothing is referencing the socket at this point.
>
> Signed-off-by: Nguyen Dinh Phi <[email protected]>
> Reported-by: [email protected]
> ---
> net/bluetooth/hci_sock.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel