Patch "netrom: Fix a memory leak in nr_heartbeat_expiry()" has been added to the 5.15-stable tree

[Sasha Levin <sashal@xxxxxxxxxx>]

This is a note to let you know that I've just added the patch titled

    netrom: Fix a memory leak in nr_heartbeat_expiry()

to the 5.15-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     netrom-fix-a-memory-leak-in-nr_heartbeat_expiry.patch
and it can be found in the queue-5.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit facf7a7b9bdf641c24a9b67401aedf7f1ac4c85b
Author: Gavrilov Ilia <Ilia.Gavrilov@xxxxxxxxxxx>
Date:   Thu Jun 13 08:23:00 2024 +0000

    netrom: Fix a memory leak in nr_heartbeat_expiry()
    
    [ Upstream commit 0b9130247f3b6a1122478471ff0e014ea96bb735 ]
    
    syzbot reported a memory leak in nr_create() [0].
    
    Commit 409db27e3a2e ("netrom: Fix use-after-free of a listening socket.")
    added sock_hold() to the nr_heartbeat_expiry() function, where
    a) a socket has a SOCK_DESTROY flag or
    b) a listening socket has a SOCK_DEAD flag.
    
    But in the case "a," when the SOCK_DESTROY flag is set, the file descriptor
    has already been closed and the nr_release() function has been called.
    So it makes no sense to hold the reference count because no one will
    call another nr_destroy_socket() and put it as in the case "b."
    
    nr_connect
      nr_establish_data_link
        nr_start_heartbeat
    
    nr_release
      switch (nr->state)
      case NR_STATE_3
        nr->state = NR_STATE_2
        sock_set_flag(sk, SOCK_DESTROY);
    
                            nr_rx_frame
                              nr_process_rx_frame
                                switch (nr->state)
                                case NR_STATE_2
                                  nr_state2_machine()
                                    nr_disconnect()
                                      nr_sk(sk)->state = NR_STATE_0
                                      sock_set_flag(sk, SOCK_DEAD)
    
                            nr_heartbeat_expiry
                              switch (nr->state)
                              case NR_STATE_0
                                if (sock_flag(sk, SOCK_DESTROY) ||
                                   (sk->sk_state == TCP_LISTEN
                                     && sock_flag(sk, SOCK_DEAD)))
                                   sock_hold()  // ( !!! )
                                   nr_destroy_socket()
    
    To fix the memory leak, let's call sock_hold() only for a listening socket.
    
    Found by InfoTeCS on behalf of Linux Verification Center
    (linuxtesting.org) with Syzkaller.
    
    [0]: https://syzkaller.appspot.com/bug?extid=d327a1f3b12e1e206c16
    
    Reported-by: syzbot+d327a1f3b12e1e206c16@xxxxxxxxxxxxxxxxxxxxxxxxx
    Closes: https://syzkaller.appspot.com/bug?extid=d327a1f3b12e1e206c16
    Fixes: 409db27e3a2e ("netrom: Fix use-after-free of a listening socket.")
    Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@xxxxxxxxxxx>
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/netrom/nr_timer.c b/net/netrom/nr_timer.c
index 4e7c968cde2dc..5e3ca068f04e0 100644
--- a/net/netrom/nr_timer.c
+++ b/net/netrom/nr_timer.c
@@ -121,7 +121,8 @@ static void nr_heartbeat_expiry(struct timer_list *t)
 		   is accepted() it isn't 'dead' so doesn't get removed. */
 		if (sock_flag(sk, SOCK_DESTROY) ||
 		    (sk->sk_state == TCP_LISTEN && sock_flag(sk, SOCK_DEAD))) {
-			sock_hold(sk);
+			if (sk->sk_state == TCP_LISTEN)
+				sock_hold(sk);
 			bh_unlock_sock(sk);
 			nr_destroy_socket(sk);
 			goto out;