[PATCH 4.14/4.19/5.4/5.10/5.15 0/1] Bluetooth: hci_sock: purge socket queues in the destruct() callback

[PATCH 4.14/4.19/5.4/5.10/5.15 0/1] Bluetooth: hci_sock: purge socket queues in the destruct() callback

[Fedor Pchelkin]

Syzkaller reports a memory leak in mgmt_cmd_complete(). The issue can be
triggered on 4.14/4.19/5.4/5.10/5.15 stable branches. The following fixing
patch can be cleanly applied to them.

[PATCH 4.14/4.19/5.4/5.10/5.15 1/1] Bluetooth: hci_sock: purge socket queues in the destruct() callback

[Fedor Pchelkin]

From: Nguyen Dinh Phi <phind.uet@gmail.com>

commit 709fca500067524381e28a5f481882930eebac88 upstream.

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 <phind.uet@gmail.com>
Reported-by: syzbot+4c4ffd1e1094dae61035@syzkaller.appspotmail.com
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
 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 f1128c2134f0..3f92a21cabe8 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -888,10 +888,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;
@@ -2012,6 +2008,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,
@@ -2065,6 +2067,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.34.1

RE: Bluetooth: hci_sock: purge socket queues in the destruct() callback

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: net/bluetooth/hci_sock.c:888
error: net/bluetooth/hci_sock.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

Re: [PATCH 4.14/4.19/5.4/5.10/5.15 1/1] Bluetooth: hci_sock: purge socket queues in the destruct() callback

[Greg Kroah-Hartman]

On Thu, Mar 09, 2023 at 09:12:51PM +0300, Fedor Pchelkin wrote:
> From: Nguyen Dinh Phi <phind.uet@gmail.com>
> 
> commit 709fca500067524381e28a5f481882930eebac88 upstream.
> 
> 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 <phind.uet@gmail.com>
> Reported-by: syzbot+4c4ffd1e1094dae61035@syzkaller.appspotmail.com
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
>  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 f1128c2134f0..3f92a21cabe8 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -888,10 +888,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;
> @@ -2012,6 +2008,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,
> @@ -2065,6 +2067,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.34.1
> 

Now queued up, thanks.

greg k-h