From patchwork Thu Jun 13 08:23:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gavrilov Ilia X-Patchwork-Id: 13696344 X-Patchwork-Delegate: kuba@kernel.org Received: from mx0.infotecs.ru (mx0.infotecs.ru [91.244.183.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4013713D51F; Thu, 13 Jun 2024 08:33:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.244.183.115 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718267588; cv=none; b=c31cFa9QTV3dR+U/cyUtoKflU94J29rG9lrt3NIALN13uFrg6atMkYBBU5lDM/Yf1SnPBHdKqE8lu7wyYqbYRCWGRy4puy7xRPnRS2KCvj7EyaRroNkbfHgDsaXPLGfPZtele/mGJ0j3FF1N99BtQAGGfntwkMnAGz+wc814gh8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718267588; c=relaxed/simple; bh=WBaYJfUtlcUcFAQlqDG8THMVEzZuOrVh6+fK22tTrDw=; h=From:To:CC:Subject:Date:Message-ID:Content-Type:MIME-Version; b=SUvxC4FPHCwjy78M0ibR01D0BdNUK4jT8qXWEoELxyM/3MZ83IXEZsUXPhemfJ9FtuQT8HyRnwo+0UvKRVose0nDQ/arZisiNTRn1HLwKfa06vU56laYs5TSFwao5SESkaTyayYD12CyJFo7s2GarbTaBP17x3bsBAcD1zk5DBA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=infotecs.ru; spf=pass smtp.mailfrom=infotecs.ru; dkim=pass (1024-bit key) header.d=infotecs.ru header.i=@infotecs.ru header.b=oVuF5vp6; arc=none smtp.client-ip=91.244.183.115 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=infotecs.ru Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infotecs.ru Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=infotecs.ru header.i=@infotecs.ru header.b="oVuF5vp6" Received: from mx0.infotecs-nt (localhost [127.0.0.1]) by mx0.infotecs.ru (Postfix) with ESMTP id 651B81210842; Thu, 13 Jun 2024 11:23:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 mx0.infotecs.ru 651B81210842 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=infotecs.ru; s=mx; t=1718266981; bh=oo8drzFyG4Hzb+eqS7LNMdqVQKwL1Fiq/olKasRjJWo=; h=From:To:CC:Subject:Date:From; b=oVuF5vp65gdnBm1j1P3aukOUOAOR1R1O/DYw4oebM+tSOvkj0dMXz7lWNMIqwMGD6 VG6aBG6thXtW6oFsTfTTQYcCjI8u3QxrAeuOOWKiabMe45Amneka5+vxg069uIGBf4 nPE6cAH2MvFTDsWIWiU822JMDf7X3+kZVANm/U3k= Received: from msk-exch-01.infotecs-nt (msk-exch-01.infotecs-nt [10.0.7.191]) by mx0.infotecs-nt (Postfix) with ESMTP id 6106C30633BB; Thu, 13 Jun 2024 11:23:01 +0300 (MSK) From: Gavrilov Ilia To: Ralf Baechle CC: "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Kuniyuki Iwashima , "linux-hams@vger.kernel.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "lvc-project@linuxtesting.org" , "syzbot+d327a1f3b12e1e206c16@syzkaller.appspotmail.com" Subject: [PATCH net] netrom: Fix a memory leak in nr_heartbeat_expiry() Thread-Topic: [PATCH net] netrom: Fix a memory leak in nr_heartbeat_expiry() Thread-Index: AQHavWrnUklO6dAj90uZJ5G3G/q5DQ== Date: Thu, 13 Jun 2024 08:23:00 +0000 Message-ID: <20240613082300.294668-1-Ilia.Gavrilov@infotecs.ru> Accept-Language: ru-RU, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-exclaimer-md-config: 208ac3cd-1ed4-4982-a353-bdefac89ac0a Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-KLMS-Rule-ID: 5 X-KLMS-Message-Action: clean X-KLMS-AntiSpam-Status: not scanned, disabled by settings X-KLMS-AntiSpam-Interceptor-Info: not scanned X-KLMS-AntiPhishing: Clean, bases: 2024/06/13 06:55:00 X-KLMS-AntiVirus: Kaspersky Security for Linux Mail Server, version 8.0.3.30, bases: 2024/06/13 04:40:00 #25585389 X-KLMS-AntiVirus-Status: Clean, skipped X-Patchwork-Delegate: kuba@kernel.org 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@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=d327a1f3b12e1e206c16 Fixes: 409db27e3a2e ("netrom: Fix use-after-free of a listening socket.") Signed-off-by: Gavrilov Ilia --- net/netrom/nr_timer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/netrom/nr_timer.c b/net/netrom/nr_timer.c index 4e7c968cde2d..5e3ca068f04e 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;