From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752588AbdLANV0 (ORCPT ); Fri, 1 Dec 2017 08:21:26 -0500 Received: from mail.windriver.com ([147.11.1.11]:37791 "EHLO mail.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751874AbdLANVZ (ORCPT ); Fri, 1 Dec 2017 08:21:25 -0500 Subject: Re: [PATCH net v2] tipc: call tipc_rcv() only if bearer is up in tipc_udp_recv() To: Tommi Rantala , Jon Maloy , "David S. Miller" , , , References: <20171129104842.30781-1-tommi.t.rantala@nokia.com> <213f64e0-d5c3-511f-b83a-c468c399abc1@windriver.com> From: Ying Xue Message-ID: <840ce8d1-7271-5189-4cfa-e40f4a0a4172@windriver.com> Date: Fri, 1 Dec 2017 21:18:51 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [128.224.155.90] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/30/2017 08:32 PM, Tommi Rantala wrote: >> In my opinion, the real root cause of the issue is because we too early >> set a not-yet-initialized bearer instance to ub->bearer through >> rcu_assign_pointer(ub->bearer, b) in tipc_udp_enable(). Instead if we >> assign the bearer pointer at the end of tipc_udp_enable() where the >> bearer has been completed the initialization, the issue would be avoided. > Hi, sorry, I fail to see how that helps. > > bearer->tolerance is only initialized in tipc_enable_bearer() after > calling m->enable_media() ie. tipc_udp_enable(). > > So even if we do "rcu_assign_pointer(ub->bearer, b)" later in > tipc_udp_enable(), bearer->tolerance will still be uninitialized, and > the crash can happen. Sorry, I missed the point that b->tolerance is not uninitialized when we assign bearer pointer to ub->bearer later. But in my view the issue happened is because we enable media too early. So it's better to change the code as belows: diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c index 47ec121..ec6f02a 100644 --- a/net/tipc/bearer.c +++ b/net/tipc/bearer.c @@ -320,19 +320,18 @@ static int tipc_enable_bearer(struct net *net, const char *name, strcpy(b->name, name); b->media = m; - res = m->enable_media(net, b, attr); - if (res) { - pr_warn("Bearer <%s> rejected, enable failure (%d)\n", - name, -res); - return -EINVAL; - } - b->identity = bearer_id; b->tolerance = m->tolerance; b->window = m->window; b->domain = disc_domain; b->net_plane = bearer_id + 'A'; b->priority = priority; + res = m->enable_media(net, b, attr); + if (res) { + pr_warn("Bearer <%s> rejected, enable failure (%d)\n", + name, -res); + return -EINVAL; + } test_and_set_bit_lock(0, &b->up);