From patchwork Mon Jun 4 16:23:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gi-Oh Kim X-Patchwork-Id: 10446991 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3797E60284 for ; Mon, 4 Jun 2018 16:24:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 217C628B93 for ; Mon, 4 Jun 2018 16:24:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 15F0728CDC; Mon, 4 Jun 2018 16:24:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 83D4228B93 for ; Mon, 4 Jun 2018 16:24:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750913AbeFDQYN (ORCPT ); Mon, 4 Jun 2018 12:24:13 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:51857 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751336AbeFDQYC (ORCPT ); Mon, 4 Jun 2018 12:24:02 -0400 Received: by mail-wm0-f65.google.com with SMTP id r15-v6so15732347wmc.1 for ; Mon, 04 Jun 2018 09:24:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=profitbricks-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=qBIZCU+Sl2j15miUkhO7PaGhg73ZDqPH2XEBqSJ/jHA=; b=dvfwz+Il4k0UdcOh5+fRdYbRL0gj6iUpoiICW+OhjwXb34IitDPqa1YZaF0wefZAhK 1u32T3Cuhu1yIpXi2LhylQeBYoCYm7bgw98UYM0H6j0uZoFuCdiKfAJpWK8/xXfFOsWP ZwfMLADRs9ffGhkSTbZmP7QApKl6Tko9TF/06dP4ywIu8RKQqe7If3242iwElHftJI3M HLR7VqOlcLXBd6djS3WvPnUNSZUeP2j17NWjPK5xB1KTZerZ5Q6RUc9ZaRInJSvguniq 7d7eKnHezmqp5nJIDT3/FS5V6z6Mb/4/MPAW/VJNFnn6xNH5EcbjeNekfo6h2YbbMjGl 7DCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=qBIZCU+Sl2j15miUkhO7PaGhg73ZDqPH2XEBqSJ/jHA=; b=XKKJmY611dr4603N57Nzokz01y3gEs/D1dii2TNVOmkybK2B2cdluWKsMZ3QeTyopk nphD03mhjQZuT7D9ipf8Soheu7JlRDTIr33g2/6nu+kidzHwo9UY7MilCbpy1jsCtgl4 CRwENwwPcYQ/EB/1my1o6Vt+oeHcJ//2NyMEmB54gCJV8wO16E+JfjgYkBne7Gj6MCyF epfoQuFNfe+MM0oTVc+PXXtoTOnqe0fGMVJef/518r3Fpl+AXCbMmxYo2BjH7gOUd51O eENYwH3RWZNUq9Sh5XrybzqvXFQRidpDiAqH2gVR1CYQZOt69Il6B9GeLJ5Kts+TQp8Z o3aA== X-Gm-Message-State: APt69E04H/DI/3JNi9dk0BwiHAS5HHh5OZig4VvlsHBrWMxEJ1iV2Z9K 4vVRQgqdaJQ6dRWrlAIwl4d5vCpnw3DqCDqbcphOhg== X-Google-Smtp-Source: ADUXVKK5xqKF4HzGlJN8Lm8xM4mizQYK7MmmTjZjgeHythRiKvz7F/Jyb/nfBi54Rwb91oOZ0V5/11sgkhqgoWFzmLk= X-Received: by 2002:a50:b2e1:: with SMTP id p88-v6mr3072167edd.207.1528129441035; Mon, 04 Jun 2018 09:24:01 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a50:de8e:0:0:0:0:0 with HTTP; Mon, 4 Jun 2018 09:23:20 -0700 (PDT) In-Reply-To: <20180601183144.17374-1-xiyou.wangcong@gmail.com> References: <20180601183144.17374-1-xiyou.wangcong@gmail.com> From: Gi-Oh Kim Date: Mon, 4 Jun 2018 18:23:20 +0200 Message-ID: Subject: Re: [PATCH] infiniband: fix a possible use-after-free bug To: Cong Wang Cc: linux-kernel@vger.kernel.org, noamr@beyondsecurity.com, Sean Hefty , Doug Ledford , Jason Gunthorpe , linux-rdma@vger.kernel.org Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jun 1, 2018 at 8:31 PM, Cong Wang wrote: > ucma_process_join() will free the new allocated "mc" struct, > if there is any error after that, especially the copy_to_user(). > > But in parallel, ucma_leave_multicast() could find this "mc" > through idr_find() before ucma_process_join() frees it, since it > is already published. > > So "mc" could be used in ucma_leave_multicast() after it is been > allocated and freed in ucma_process_join(), since we don't refcnt > it. > > Fix this by separating "publish" from ID allocation, so that we > can get an ID first and publish it later after copy_to_user(). > > Fixes c8f6a362bf3e ("RDMA/cma: Add multicast communication support") > Reported-by: Noam Rathaus > Cc: Sean Hefty > Cc: Doug Ledford > Cc: Jason Gunthorpe > Cc: linux-rdma@vger.kernel.org > Signed-off-by: Cong Wang > --- > drivers/infiniband/core/ucma.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index eab43b17e9cf..ec8fb289621f 100644 > --- a/drivers/infiniband/core/ucma.c > +++ b/drivers/infiniband/core/ucma.c > @@ -235,7 +235,7 @@ static struct ucma_multicast* ucma_alloc_multicast(struct ucma_context *ctx) > return NULL; > > mutex_lock(&mut); > - mc->id = idr_alloc(&multicast_idr, mc, 0, 0, GFP_KERNEL); > + mc->id = idr_alloc(&multicast_idr, NULL, 0, 0, GFP_KERNEL); > mutex_unlock(&mut); > if (mc->id < 0) > goto error; > @@ -1421,6 +1421,10 @@ static ssize_t ucma_process_join(struct ucma_file *file, > goto err3; > } > > + mutex_lock(&mut); > + idr_replace(&multicast_idr, mc, mc->id); > + mutex_unlock(&mut); > + > mutex_unlock(&file->mut); > ucma_put_ctx(ctx); > return 0; > -- > 2.13.0 > Hi, Your patch is reasonable to me. Can I ask a question for that? Could it be solved by asymmetric locking as following? if (!mc) @@ -1507,11 +1508,11 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file, if (IS_ERR(mc)) { ret = PTR_ERR(mc); + mutex_unlock(&mc->ctx->file->mut); goto out; } rdma_leave_multicast(mc->ctx->cm_id, (struct sockaddr *) &mc->addr); - mutex_lock(&mc->ctx->file->mut); ucma_cleanup_mc_events(mc); list_del(&mc->list); mutex_unlock(&mc->ctx->file->mut); diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index eab43b17e9cf..d8b256baec31 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1493,6 +1493,7 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file, if (copy_from_user(&cmd, inbuf, sizeof(cmd))) return -EFAULT; + mutex_lock(&mc->ctx->file->mut); mutex_lock(&mut); mc = idr_find(&multicast_idr, cmd.id);