From patchwork Wed Apr 11 20:52:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Maloy X-Patchwork-Id: 897407 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=ericsson.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ericsson.com header.i=@ericsson.com header.b="E6BwhRgE"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40Lx7s2PRFz9s16 for ; Thu, 12 Apr 2018 06:52:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932341AbeDKUwr (ORCPT ); Wed, 11 Apr 2018 16:52:47 -0400 Received: from sessmg23.ericsson.net ([193.180.251.45]:46927 "EHLO sessmg23.ericsson.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755493AbeDKUwn (ORCPT ); Wed, 11 Apr 2018 16:52:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1523479961; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=Sj1z4tOBQWBsHVIDQPuHFb72EF7qlhLQptlqJODvjAw=; b=E6BwhRgETMpwuEHP3GwRWyuEReh971wA4SuA/3GYnnksUPbCegLpxqwvWy0XaLl0 w2PpYokr4h8vK6TpTvWhBzQ0nnVVjDv0C6bm8nrk1K4wp1n2O7sYw4HJslzDLy2B PR/sRBM/ABHWd/g8gVO4zfL49KqET/riIvdbdbgIgKQ=; X-AuditID: c1b4fb2d-e31ff700000073d9-99-5ace7598084b Received: from ESESSHC011.ericsson.se (Unknown_Domain [153.88.183.51]) by sessmg23.ericsson.net (Symantec Mail Security) with SMTP id EE.1A.29657.8957ECA5; Wed, 11 Apr 2018 22:52:41 +0200 (CEST) Received: from daly.lab.linux.ericsson.se (10.35.28.123) by ESESSHC011.ericsson.se (153.88.183.51) with Microsoft SMTP Server (TLS) id 14.3.382.0; Wed, 11 Apr 2018 22:52:40 +0200 From: Jon Maloy To: , CC: , , , , , , Subject: [net 1/1] tipc: fix unbalanced reference counter Date: Wed, 11 Apr 2018 22:52:09 +0200 Message-ID: <1523479929-28161-1-git-send-email-jon.maloy@ericsson.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 X-Originating-IP: [10.35.28.123] X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrKLMWRmVeSWpSXmKPExsUyM2K7se7M0nNRBp1XOSxuNPQwW8w538Ji 8fbVLHaLYwvELLacz7K40n6W3eLx9evMDuweW1beZPJ4d4XNY/eCz0wenzfJeazfspUpgDWK yyYlNSezLLVI3y6BK+N71xeWggsyFdduTGdsYPwo3sXIySEhYCKx9G4bYxcjF4eQwBFGiR39 m9kgnG2MEpc/bWYGqWIT0JB4Oa2DEcQWETCWeLWykwmkiFngE6NEy8pnbCAJYQEriQuTp4AV sQioSsw8tRTI5uDgFXCTaOtjgtgmJ3H++E+wmbwCghInZz5hAbGZBSQkDr54ARYXElCWmPth GlS9gsSHWcvYJjDyzULSMgtJywJGplWMosWpxcW56UbGeqlFmcnFxfl5enmpJZsYgYF5cMtv 3R2Mq187HmIU4GBU4uGtCjgXJcSaWFZcmXuIUYKDWUmE9+jvs1FCvCmJlVWpRfnxRaU5qcWH GKU5WJTEefVW7YkSEkhPLEnNTk0tSC2CyTJxcEo1MIaze9v7+HpqLpZQ9liRz77A1vnmlmVH 2haV+n2e3vdztnzEls73bAXq+XeSbcJ9jD4+EwtR2johRm7Vopt1+7/4lFnNZcj3Lf+rs11W 5IDbPjYj3zlHzO+fzDKyVlh58d3eW+/OXs1cUjOtqbzqilh28K+Hkh6c5i9V3dU/3zOp3HJg /XxZfyWW4oxEQy3mouJEAAiAwvlIAgAA Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When a topology subscription is created, we may encounter (or KASAN may provoke) a failure to create a corresponding service instance in the binding table. Instead of letting the tipc_nametbl_subscribe() report the failure back to the caller, the function just makes a warning printout and returns, without incrementing the subscription reference counter as expected by the caller. This makes the caller believe that the subscription was successful, so it will at a later moment try to unsubscribe the item. This involves a sub_put() call. Since the reference counter never was incremented in the first place, we get a premature delete of the subscription item, followed by a "use-after-free" warning. We fix this by adding a return value to tipc_nametbl_subscribe() and make the caller aware of the failure to subscribe. This bug seems to always have been around, but this fix only applies back to the commit shown below. Given the low risk of this happening we believe this to be sufficient. Fixes: commit 218527fe27ad ("tipc: replace name table service range array with rb tree") Reported-by: syzbot+aa245f26d42b8305d157@syzkaller.appspotmail.com Signed-off-by: Jon Maloy --- net/tipc/name_table.c | 5 ++++- net/tipc/name_table.h | 2 +- net/tipc/subscr.c | 5 ++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c index b1fe209..4068eaa 100644 --- a/net/tipc/name_table.c +++ b/net/tipc/name_table.c @@ -665,13 +665,14 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, /** * tipc_nametbl_subscribe - add a subscription object to the name table */ -void tipc_nametbl_subscribe(struct tipc_subscription *sub) +bool tipc_nametbl_subscribe(struct tipc_subscription *sub) { struct name_table *nt = tipc_name_table(sub->net); struct tipc_net *tn = tipc_net(sub->net); struct tipc_subscr *s = &sub->evt.s; u32 type = tipc_sub_read(s, seq.type); struct tipc_service *sc; + bool res = true; spin_lock_bh(&tn->nametbl_lock); sc = tipc_service_find(sub->net, type); @@ -685,8 +686,10 @@ void tipc_nametbl_subscribe(struct tipc_subscription *sub) pr_warn("Failed to subscribe for {%u,%u,%u}\n", type, tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper)); + res = false; } spin_unlock_bh(&tn->nametbl_lock); + return res; } /** diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h index 4b14fc2..0febba4 100644 --- a/net/tipc/name_table.h +++ b/net/tipc/name_table.h @@ -126,7 +126,7 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type, struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type, u32 lower, u32 upper, u32 node, u32 key); -void tipc_nametbl_subscribe(struct tipc_subscription *s); +bool tipc_nametbl_subscribe(struct tipc_subscription *s); void tipc_nametbl_unsubscribe(struct tipc_subscription *s); int tipc_nametbl_init(struct net *net); void tipc_nametbl_stop(struct net *net); diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index b7d80bc..f340e53 100644 --- a/net/tipc/subscr.c +++ b/net/tipc/subscr.c @@ -153,7 +153,10 @@ struct tipc_subscription *tipc_sub_subscribe(struct net *net, memcpy(&sub->evt.s, s, sizeof(*s)); spin_lock_init(&sub->lock); kref_init(&sub->kref); - tipc_nametbl_subscribe(sub); + if (!tipc_nametbl_subscribe(sub)) { + kfree(sub); + return NULL; + } timer_setup(&sub->timer, tipc_sub_timeout, 0); timeout = tipc_sub_read(&sub->evt.s, timeout); if (timeout != TIPC_WAIT_FOREVER)