From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-1562242-1516941255-2-13783072480266157545 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, HEADER_FROM_DIFFERENT_DOMAINS 0.25, ME_NOAUTH 0.01, RCVD_IN_DNSWL_HI -5, T_RP_MATCHES_RCVD -0.01, LANGUAGES unknown, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.132.180.67', Host='vger.kernel.org', Country='US', FromHeader='com', MailFrom='org', XOriginatingCountry='UNK' X-Spam-charsets: plain='us-ascii' X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: stable-owner@vger.kernel.org ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1516941254; b=mlJZ5R7GSN/RxgIe1K+mzqSRKOhjh12sLN7G9q5jstDgIlJ ZVdbdaxMFluF5tuPxX+TcRanqr5nR97HzDbcCCsncbkKp6KBCaBxDNlkoVtjUk6/ azfFdFVsXPRxKqlE3dMmp4r0Xtr35cy7xHgGL9d2NZkpJaEOuuiYq2QhAZO2v0EM I/tckEJQQ9VguI8bpnTPTiPOio901rnR//46carNeImDvs6wVsJC46QSy7/Gltvs 84zgOz3Miy8jGdZnKsPACeYtwiWRHlX1sQkdP2GhJqhdBMDxU6DE+w823VAoaHc4 u3wr7uRdQAvK+ywMBk69WprBOEEcvR5tr5EscSw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=from:to:cc:subject:date:message-id :references:in-reply-to:content-type:content-id :content-transfer-encoding:mime-version:sender:list-id; s= arctest; t=1516941254; bh=JGvh2oC/nHS1hV7qwfQOonGrECOcVQCaVQrA9Y b1Rdo=; b=acauk1Ud4pMHIXK3mGCi+ZbsS/r+gxys4f2POKbgJANPtC9gE2aDrc 7mofY0LXPT2AeW4Bp/ez+tvkbY0NuGK5W29bmvJQFRD4Tjcgz8DbMAxIEMTt7T/w WFaVeM97lMInCDmEjR/YZRkQdiFt7fnW0U/Sf6m6/tVvMl8hsnlJurCD2nVeBrA+ cEdbBksVZc6fKSlX8ajJsFVKR1N9uZrvh5Wb5ACeT8t3VqUFmvWoAn74/lvFq3n3 RoiGC4eSp0q6bezVowoUh9LEzJoz97nX8UvxyZXOls4encIcpqRW44pCAi7yrM29 rFtoev2T8V5maFCPGynOV8gq0QVDT1dg== ARC-Authentication-Results: i=1; mx3.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=intel.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=stable-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=intel.com header.result=pass header_is_org_domain=yes Authentication-Results: mx3.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=intel.com; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=stable-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=intel.com header.result=pass header_is_org_domain=yes Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751324AbeAZEeK convert rfc822-to-8bit (ORCPT ); Thu, 25 Jan 2018 23:34:10 -0500 Received: from mga01.intel.com ([192.55.52.88]:30414 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbeAZEeK (ORCPT ); Thu, 25 Jan 2018 23:34:10 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,415,1511856000"; d="scan'208";a="196378669" From: "Dilger, Andreas" To: "Eremin, Dmitry" CC: Greg Kroah-Hartman , "devel@driverdev.osuosl.org" , "Drokin, Oleg" , James Simmons , "Linux Kernel Mailing List" , Lustre Development List , "stable@vger.kernel.org" Subject: Re: [PATCH v4] staging: lustre: separate a connection destroy from free struct kib_conn Thread-Topic: [PATCH v4] staging: lustre: separate a connection destroy from free struct kib_conn Thread-Index: AQHTleOd5DtaQm4MTEqqYegtFgN8k6OGGCEA Date: Fri, 26 Jan 2018 04:34:07 +0000 Message-ID: <33F9CC96-70A5-4B03-9434-64DD37532B29@intel.com> References: <20180124142954.GA4658@kroah.com> <1516888264-26273-1-git-send-email-dmitry.eremin@intel.com> In-Reply-To: <1516888264-26273-1-git-send-email-dmitry.eremin@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.103.2] Content-Type: text/plain; charset="us-ascii" Content-ID: <8C8272F027432D4BBA9FC1D704283965@intel.com> Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org X-Mailing-List: stable@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Jan 25, 2018, at 06:51, Eremin, Dmitry wrote: > > The logic of the original commit 4d99b2581eff ("staging: lustre: avoid > intensive reconnecting for ko2iblnd") was assumed conditional free of > struct kib_conn if the second argument free_conn in function > kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) is true. > But this hunk of code was dropped from original commit. As result the logic > works wrong and current code use struct kib_conn after free. > >> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c >> 3317 kiblnd_destroy_conn(conn, !peer); >> ^^^^ Freed always (but should be conditionally) >> 3318 >> 3319 spin_lock_irqsave(lock, flags); >> 3320 if (!peer) >> 3321 continue; >> 3322 >> 3323 conn->ibc_peer = peer; >> ^^^^^^^^^^^^^^ Use after free >> 3324 if (peer->ibp_reconnected < KIB_RECONN_HIGH_RACE) >> 3325 list_add_tail(&conn->ibc_list, >> ^^^^^^^^^^^^^^ Use after free >> 3326 &kiblnd_data.kib_reconn_list); >> 3327 else >> 3328 list_add_tail(&conn->ibc_list, >> ^^^^^^^^^^^^^^ Use after free >> 3329 &kiblnd_data.kib_reconn_wait); > > To avoid confusion this fix moved the freeing a struct kib_conn outside of > the function kiblnd_destroy_conn() and free as it was intended in original > commit. > > Cc: # v4.6 > Fixes: 4d99b2581eff ("staging: lustre: avoid intensive reconnecting for ko2iblnd") > Signed-off-by: Dmitry Eremin Reviewed-by: Andreas Dilger > --- > Changes in v4: > - fixed the issue with use after free by moving the freeing a struct > kib_conn outside of the function kiblnd_destroy_conn() > > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 7 +++---- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h | 2 +- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 6 ++++-- > 3 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > index 2ebc484385b3..ec84edfda271 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > @@ -824,14 +824,15 @@ struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, struct rdma_cm_id *cm > return conn; > > failed_2: > - kiblnd_destroy_conn(conn, true); > + kiblnd_destroy_conn(conn); > + kfree(conn); > failed_1: > kfree(init_qp_attr); > failed_0: > return NULL; > } > > -void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) > +void kiblnd_destroy_conn(struct kib_conn *conn) > { > struct rdma_cm_id *cmid = conn->ibc_cmid; > struct kib_peer *peer = conn->ibc_peer; > @@ -889,8 +890,6 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn) > rdma_destroy_id(cmid); > atomic_dec(&net->ibn_nconns); > } > - > - kfree(conn); > } > > int kiblnd_close_peer_conns_locked(struct kib_peer *peer, int why) > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h > index 171eced213f8..b18911d09e9a 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h > @@ -1016,7 +1016,7 @@ int kiblnd_close_stale_conns_locked(struct kib_peer *peer, > struct kib_conn *kiblnd_create_conn(struct kib_peer *peer, > struct rdma_cm_id *cmid, > int state, int version); > -void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn); > +void kiblnd_destroy_conn(struct kib_conn *conn); > void kiblnd_close_conn(struct kib_conn *conn, int error); > void kiblnd_close_conn_locked(struct kib_conn *conn, int error); > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > index 9b3328c5d1e7..b3e7f28eb978 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > @@ -3314,11 +3314,13 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid, > spin_unlock_irqrestore(lock, flags); > dropped_lock = 1; > > - kiblnd_destroy_conn(conn, !peer); > + kiblnd_destroy_conn(conn); > > spin_lock_irqsave(lock, flags); > - if (!peer) > + if (!peer) { > + kfree(conn); > continue; > + } > > conn->ibc_peer = peer; > if (peer->ibp_reconnected < KIB_RECONN_HIGH_RACE) > -- > 1.8.3.1 > Cheers, Andreas -- Andreas Dilger Lustre Principal Architect Intel Corporation