Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Anil Altinay]

Hi,

Can you please backport the following commit to 5.15 stable?
Title: "af_unix: Fix memory leaks of the whole sk due to OOB skb."
Commit SHA: 7a62ed61367b8fd01bae1e18e30602c25060d824

This commit fixes "314001f0bf927015e459c9d387d62a231fe93af3" which
landed on "tags/v5.15-rc1~157^2~305".

Best,
Anil

Re: Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Greg KH]

On Mon, Oct 31, 2022 at 11:25:32AM -0700, Anil Altinay wrote:
> Hi,
> 
> Can you please backport the following commit to 5.15 stable?
> Title: "af_unix: Fix memory leaks of the whole sk due to OOB skb."
> Commit SHA: 7a62ed61367b8fd01bae1e18e30602c25060d824
> 
> This commit fixes "314001f0bf927015e459c9d387d62a231fe93af3" which
> landed on "tags/v5.15-rc1~157^2~305".

As this commit does not apply cleanly on the 5.15.y tree, how was it
tested?

Can you provide a working version of this change so that we can apply
it?

thanks,

greg k-h

Re: Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Anil Altinay]

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

Please see attached patch for backported version with the commit
message updated with the upstream commit SHA. I also applied this
patch to our kernel and it built fine. Hopefully, it is good to go
this time.


On Tue, Nov 1, 2022 at 12:33 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Oct 31, 2022 at 11:25:32AM -0700, Anil Altinay wrote:
> > Hi,
> >
> > Can you please backport the following commit to 5.15 stable?
> > Title: "af_unix: Fix memory leaks of the whole sk due to OOB skb."
> > Commit SHA: 7a62ed61367b8fd01bae1e18e30602c25060d824
> >
> > This commit fixes "314001f0bf927015e459c9d387d62a231fe93af3" which
> > landed on "tags/v5.15-rc1~157^2~305".
>
> As this commit does not apply cleanly on the 5.15.y tree, how was it
> tested?
>
> Can you provide a working version of this change so that we can apply
> it?
>
> thanks,
>
> greg k-h

[-- Attachment #2: 0001-af_unix-Fix-memory-leaks-of-the-whole-sk-due-to-OOB-.patch --]
[-- Type: application/x-patch, Size: 3690 bytes --]

Re: Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Greg KH]

On Wed, Nov 02, 2022 at 07:59:24PM -0700, Anil Altinay wrote:
> Please see attached patch for backported version with the commit
> message updated with the upstream commit SHA. I also applied this
> patch to our kernel and it built fine. Hopefully, it is good to go
> this time.

You forgot to sign off on the patch :(

Re: Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Anil Altinay]

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

Ops, I am sorry. It is my first time sending a patch to the stable
tree. I added "Signed-off-by: Anil Altinay <aaltinay@google.com>" at
the end of the Signed-off list. Hopefully, it is good to go this time
:) Thanks for your patience Greg!

On Wed, Nov 2, 2022 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Nov 02, 2022 at 07:59:24PM -0700, Anil Altinay wrote:
> > Please see attached patch for backported version with the commit
> > message updated with the upstream commit SHA. I also applied this
> > patch to our kernel and it built fine. Hopefully, it is good to go
> > this time.
>
> You forgot to sign off on the patch :(
>

[-- Attachment #2: 0001-af_unix-Fix-memory-leaks-of-the-whole-sk-due-to-OOB- (3).patch --]
[-- Type: application/octet-stream, Size: 3740 bytes --]

From 0be0f47bad0c736251de41b64e48782f6b6a0f2d Mon Sep 17 00:00:00 2001
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Thu, 29 Sep 2022 08:52:04 -0700
Subject: [PATCH] af_unix: Fix memory leaks of the whole sk due to OOB skb.

[ Upstream commit 7a62ed61367b8fd01bae1e18e30602c25060d824 ]

syzbot reported a sequence of memory leaks, and one of them indicated we
failed to free a whole sk:

  unreferenced object 0xffff8880126e0000 (size 1088):
    comm "syz-executor419", pid 326, jiffies 4294773607 (age 12.609s)
    hex dump (first 32 bytes):
      00 00 00 00 00 00 00 00 7d 00 00 00 00 00 00 00  ........}.......
      01 00 07 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
    backtrace:
      [<000000006fefe750>] sk_prot_alloc+0x64/0x2a0 net/core/sock.c:1970
      [<0000000074006db5>] sk_alloc+0x3b/0x800 net/core/sock.c:2029
      [<00000000728cd434>] unix_create1+0xaf/0x920 net/unix/af_unix.c:928
      [<00000000a279a139>] unix_create+0x113/0x1d0 net/unix/af_unix.c:997
      [<0000000068259812>] __sock_create+0x2ab/0x550 net/socket.c:1516
      [<00000000da1521e1>] sock_create net/socket.c:1566 [inline]
      [<00000000da1521e1>] __sys_socketpair+0x1a8/0x550 net/socket.c:1698
      [<000000007ab259e1>] __do_sys_socketpair net/socket.c:1751 [inline]
      [<000000007ab259e1>] __se_sys_socketpair net/socket.c:1748 [inline]
      [<000000007ab259e1>] __x64_sys_socketpair+0x97/0x100 net/socket.c:1748
      [<000000007dedddc1>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
      [<000000007dedddc1>] do_syscall_64+0x38/0x90 arch/x86/entry/common.c:80
      [<000000009456679f>] entry_SYSCALL_64_after_hwframe+0x63/0xcd

We can reproduce this issue by creating two AF_UNIX SOCK_STREAM sockets,
send()ing an OOB skb to each other, and close()ing them without consuming
the OOB skbs.

  int skpair[2];

  socketpair(AF_UNIX, SOCK_STREAM, 0, skpair);

  send(skpair[0], "x", 1, MSG_OOB);
  send(skpair[1], "x", 1, MSG_OOB);

  close(skpair[0]);
  close(skpair[1]);

Currently, we free an OOB skb in unix_sock_destructor() which is called via
__sk_free(), but it's too late because the receiver's unix_sk(sk)->oob_skb
is accounted against the sender's sk->sk_wmem_alloc and __sk_free() is
called only when sk->sk_wmem_alloc is 0.

In the repro sequences, we do not consume the OOB skb, so both two sk's
sock_put() never reach __sk_free() due to the positive sk->sk_wmem_alloc.
Then, no one can consume the OOB skb nor call __sk_free(), and we finally
leak the two whole sk.

Thus, we must free the unconsumed OOB skb earlier when close()ing the
socket.

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Anil Altinay <aaltinay@google.com>
---
 net/unix/af_unix.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 78e08e82c08c..3d20a9b923ab 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -504,12 +504,6 @@ static void unix_sock_destructor(struct sock *sk)
 
 	skb_queue_purge(&sk->sk_receive_queue);
 
-#if IS_ENABLED(CONFIG_AF_UNIX_OOB)
-	if (u->oob_skb) {
-		kfree_skb(u->oob_skb);
-		u->oob_skb = NULL;
-	}
-#endif
 	WARN_ON(refcount_read(&sk->sk_wmem_alloc));
 	WARN_ON(!sk_unhashed(sk));
 	WARN_ON(sk->sk_socket);
@@ -556,6 +550,13 @@ static void unix_release_sock(struct sock *sk, int embrion)
 
 	unix_state_unlock(sk);
 
+#if IS_ENABLED(CONFIG_AF_UNIX_OOB)
+	if (u->oob_skb) {
+		kfree_skb(u->oob_skb);
+		u->oob_skb = NULL;
+	}
+#endif
+
 	wake_up_interruptible_all(&u->peer_wait);
 
 	if (skpair != NULL) {
-- 
2.38.1.273.g43a17bfeac-goog

Re: Backporting 7a62ed61367b8fd01bae1e18e30602c25060d824 to 5.15 stable

[Greg KH]

On Wed, Nov 02, 2022 at 11:44:49PM -0700, Anil Altinay wrote:
> Ops, I am sorry. It is my first time sending a patch to the stable
> tree. I added "Signed-off-by: Anil Altinay <aaltinay@google.com>" at
> the end of the Signed-off list. Hopefully, it is good to go this time
> :) Thanks for your patience Greg!

Now queued up, thanks.

greg k-h