From patchwork Thu Mar 20 23:41:12 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 3868271 Return-Path: X-Original-To: patchwork-ocfs2-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 84866BF540 for ; Thu, 20 Mar 2014 23:42:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8CF5120213 for ; Thu, 20 Mar 2014 23:42:48 +0000 (UTC) Received: from userp1040.oracle.com (userp1040.oracle.com [156.151.31.81]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6E1D720176 for ; Thu, 20 Mar 2014 23:42:46 +0000 (UTC) Received: from acsinet22.oracle.com (acsinet22.oracle.com [141.146.126.238]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id s2KNfrhp011303 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 20 Mar 2014 23:41:54 GMT Received: from oss.oracle.com (oss-external.oracle.com [137.254.96.51]) by acsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id s2KNfnq9002902 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 20 Mar 2014 23:41:50 GMT Received: from localhost ([127.0.0.1] helo=oss.oracle.com) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1WQmar-0008Tu-Go; Thu, 20 Mar 2014 16:41:49 -0700 Received: from acsinet21.oracle.com ([141.146.126.237]) by oss.oracle.com with esmtp (Exim 4.63) (envelope-from ) id 1WQmaJ-0008Sx-8N for ocfs2-devel@oss.oracle.com; Thu, 20 Mar 2014 16:41:15 -0700 Received: from aserp1020.oracle.com (aserp1020.oracle.com [141.146.126.67]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id s2KNfER2018287 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 20 Mar 2014 23:41:14 GMT Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) by aserp1020.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id s2KNfDO3027572 for ; Thu, 20 Mar 2014 23:41:13 GMT Received: from akpm3.mtv.corp.google.com (unknown [216.239.45.95]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id E8A742B; Thu, 20 Mar 2014 23:41:12 +0000 (UTC) Date: Thu, 20 Mar 2014 16:41:12 -0700 From: Andrew Morton To: Joseph Qi Message-Id: <20140320164112.df1aa9b451527b1493eef799@linux-foundation.org> In-Reply-To: <532A6DAA.9020707@huawei.com> References: <532A6DAA.9020707@huawei.com> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu) Mime-Version: 1.0 X-Flow-Control-Info: class=Pass-to-MM reputation=ipRisk-All ip=140.211.169.12 ct-class=T1 ct-vol1=0 ct-vol2=4 ct-vol3=4 ct-risk=10 ct-spam1=0 ct-spam2=0 ct-bulk=89 rcpts=1 size=3232 X-Sendmail-CM-Score: 0.00% X-Sendmail-CM-Analysis: v=2.1 cv=A/tVYcmG c=1 sm=1 tr=0 a=5MPDoNpceV4HFXFrvkM3CQ==:117 a=5MPDoNpceV4HFXFrvkM3CQ==:17 a=1wixQNcEeJYA:10 a=NEiEQogP1MkA:10 a=kj9zAlcOel0A:10 a=Z4Rwk6OoAAAA:8 a=1XWaLZrsAAAA:8 a=ag1SF4gXAAAA:8 a=i0EeH86SAAAA:8 a=UtV5FjuMAAAA:8 a=G9U NOix0xJ6_Q5veyBcA:9 a=CjuIK1q_8ugA:10 a=hPjdaMEvmhQA:10 a=UrVzQrUuqIYA:10 X-Sendmail-CT-RefID: str=0001.0A090207.532B7C9A.002F:SCFSTAT19734153, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-Sendmail-CT-Classification: not spam Cc: guozhonghua@h3c.com, Mark Fasheh , "ocfs2-devel@oss.oracle.com" Subject: Re: [Ocfs2-devel] [PATCH] ocfs2: fix dead lock risk when kmalloc failed in dlm_query_region_handler X-BeenThere: ocfs2-devel@oss.oracle.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ocfs2-devel-bounces@oss.oracle.com Errors-To: ocfs2-devel-bounces@oss.oracle.com X-Source-IP: acsinet22.oracle.com [141.146.126.238] X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 20 Mar 2014 12:25:14 +0800 Joseph Qi wrote: > From: Zhonghua Guo > > In dlm_query_region_handler(), once kmalloc failed, it will unlock > dlm_domain_lock without lock first, then dead lock happens. > This patch adds a flag domain_locked to fix this issue. > > ... > > --- a/fs/ocfs2/dlm/dlmdomain.c > +++ b/fs/ocfs2/dlm/dlmdomain.c > @@ -1124,6 +1124,7 @@ static int dlm_query_region_handler(struct o2net_msg *msg, u32 len, > char *local = NULL; > int status = 0; > int locked = 0; > + int domain_locked = 0; > > qr = (struct dlm_query_region *) msg->buf; > > @@ -1140,6 +1141,7 @@ static int dlm_query_region_handler(struct o2net_msg *msg, u32 len, > status = -EINVAL; > > spin_lock(&dlm_domain_lock); > + domain_locked = 1; > dlm = __dlm_lookup_domain_full(qr->qr_domain, qr->qr_namelen); > if (!dlm) { > mlog(ML_ERROR, "Node %d queried hb regions on domain %s " > @@ -1171,7 +1173,8 @@ static int dlm_query_region_handler(struct o2net_msg *msg, u32 len, > bail: > if (locked) > spin_unlock(&dlm->spinlock); > - spin_unlock(&dlm_domain_lock); > + if (domain_locked) > + spin_unlock(&dlm_domain_lock); > > kfree(local); That makes the code messier, by adding another local and testing it. We can instead make the code cleaner by adopting conventional error unwinding techniques: Please review and test this then send it back at me? --- a/fs/ocfs2/dlm/dlmdomain.c~ocfs2-fix-dead-lock-risk-when-kmalloc-failed-in-dlm_query_region_handler +++ a/fs/ocfs2/dlm/dlmdomain.c @@ -1123,7 +1123,6 @@ static int dlm_query_region_handler(stru struct dlm_ctxt *dlm = NULL; char *local = NULL; int status = 0; - int locked = 0; qr = (struct dlm_query_region *) msg->buf; @@ -1132,10 +1131,8 @@ static int dlm_query_region_handler(stru /* buffer used in dlm_mast_regions() */ local = kmalloc(sizeof(qr->qr_regions), GFP_KERNEL); - if (!local) { - status = -ENOMEM; - goto bail; - } + if (!local) + return -ENOMEM; status = -EINVAL; @@ -1144,16 +1141,15 @@ static int dlm_query_region_handler(stru if (!dlm) { mlog(ML_ERROR, "Node %d queried hb regions on domain %s " "before join domain\n", qr->qr_node, qr->qr_domain); - goto bail; + goto out_domain_lock; } spin_lock(&dlm->spinlock); - locked = 1; if (dlm->joining_node != qr->qr_node) { mlog(ML_ERROR, "Node %d queried hb regions on domain %s " "but joining node is %d\n", qr->qr_node, qr->qr_domain, dlm->joining_node); - goto bail; + goto out_dlm_lock; } /* Support for global heartbeat was added in 1.1 */ @@ -1163,14 +1159,14 @@ static int dlm_query_region_handler(stru "but active dlm protocol is %d.%d\n", qr->qr_node, qr->qr_domain, dlm->dlm_locking_proto.pv_major, dlm->dlm_locking_proto.pv_minor); - goto bail; + goto out_dlm_lock; } status = dlm_match_regions(dlm, qr, local, sizeof(qr->qr_regions)); -bail: - if (locked) - spin_unlock(&dlm->spinlock); +out_dlm_lock: + spin_unlock(&dlm->spinlock); +out_domain_lock: spin_unlock(&dlm_domain_lock); kfree(local);