From patchwork Tue Sep 29 11:00:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kroah-Hartman X-Patchwork-Id: 290835 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46A9FC4727F for ; Tue, 29 Sep 2020 12:27:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E7CD62083B for ; Tue, 29 Sep 2020 12:26:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1601382420; bh=2FhpMM7YK2Frh/1hidyqqdIo9NYXx5rasKpjyI38Hmc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=xwx8ogzkINA0r89xgJUU0+kymnMnoBqcGPzt0sy9+CLcjswPKAZWzbza9nnMlADbt yssA07reEL4xPbxXbJ9KIIjJQUj5PBteIt6t1dTN8dRsMRx/vOdqT7qdMXAeDf+RRK 037fkz/R8y6K7tfrB/UQOOlExjmGV+vkOoCloBNo= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732741AbgI2M0k (ORCPT ); Tue, 29 Sep 2020 08:26:40 -0400 Received: from mail.kernel.org ([198.145.29.99]:49132 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729656AbgI2Ldc (ORCPT ); Tue, 29 Sep 2020 07:33:32 -0400 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B94CF23BCF; Tue, 29 Sep 2020 11:27:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1601378852; bh=2FhpMM7YK2Frh/1hidyqqdIo9NYXx5rasKpjyI38Hmc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=i0j/hZ8dA+Ccc3ZTMQsJQLOXA0gl8f+G7tMp10IAtFMA1wPvMHZdj3S7yXTSXPLI5 Rji4tc8x0pQSvTAcKTH13DQIb5ajuILotQvsVICv2wOaX6paptZLg/1Zv+PWE2zwMB dRTNMtbTu/ACLKjHxiSbYlTbWMR0iWaQjBlQhDiw= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Ying Xue , Jon Maloy , Thang Ngo , Tuong Lien , "David S. Miller" , Sasha Levin Subject: [PATCH 4.19 162/245] tipc: fix memory leak in service subscripting Date: Tue, 29 Sep 2020 13:00:13 +0200 Message-Id: <20200929105954.866777704@linuxfoundation.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200929105946.978650816@linuxfoundation.org> References: <20200929105946.978650816@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Tuong Lien [ Upstream commit 0771d7df819284d46cf5cfb57698621b503ec17f ] Upon receipt of a service subscription request from user via a topology connection, one 'sub' object will be allocated in kernel, so it will be able to send an event of the service if any to the user correspondingly then. Also, in case of any failure, the connection will be shutdown and all the pertaining 'sub' objects will be freed. However, there is a race condition as follows resulting in memory leak: receive-work connection send-work | | | sub-1 |<------//-------| | sub-2 |<------//-------| | | |<---------------| evt for sub-x sub-3 |<------//-------| | : : : : : : | /--------| | | | * peer closed | | | | | | | |<-------X-------| evt for sub-y | | |<===============| sub-n |<------/ X shutdown | -> orphan | | That is, the 'receive-work' may get the last subscription request while the 'send-work' is shutting down the connection due to peer close. We had a 'lock' on the connection, so the two actions cannot be carried out simultaneously. If the last subscription is allocated e.g. 'sub-n', before the 'send-work' closes the connection, there will be no issue at all, the 'sub' objects will be freed. In contrast the last subscription will become orphan since the connection was closed, and we released all references. This commit fixes the issue by simply adding one test if the connection remains in 'connected' state right after we obtain the connection lock, then a subscription object can be created as usual, otherwise we ignore it. Acked-by: Ying Xue Acked-by: Jon Maloy Reported-by: Thang Ngo Signed-off-by: Tuong Lien Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- net/tipc/topsrv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index 41f4464ac6cc5..ec9a7137d2677 100644 --- a/net/tipc/topsrv.c +++ b/net/tipc/topsrv.c @@ -407,7 +407,9 @@ static int tipc_conn_rcv_from_sock(struct tipc_conn *con) return -EWOULDBLOCK; if (ret == sizeof(s)) { read_lock_bh(&sk->sk_callback_lock); - ret = tipc_conn_rcv_sub(srv, con, &s); + /* RACE: the connection can be closed in the meantime */ + if (likely(connected(con))) + ret = tipc_conn_rcv_sub(srv, con, &s); read_unlock_bh(&sk->sk_callback_lock); if (!ret) return 0;