All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: torvalds@linux-foundation.org, jannh@google.com,
	paulmck@linux.vnet.ibm.com, bcrl@kvack.org,
	viro@zeniv.linux.org.uk, kent.overstreet@gmail.com
Cc: security@kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com, Tejun Heo <tj@kernel.org>,
	Dennis Dalessandro <dennis.dalessandro@intel.com>,
	Mike Marciniszyn <mike.marciniszyn@intel.com>,
	linux-rdma@vger.kernel.org
Subject: [PATCH 3/7] RDMAVT: Fix synchronization around percpu_ref
Date: Tue,  6 Mar 2018 09:33:12 -0800	[thread overview]
Message-ID: <20180306173316.3088458-3-tj@kernel.org> (raw)
In-Reply-To: <20180306173316.3088458-1-tj@kernel.org>

rvt_mregion uses percpu_ref for reference counting and RCU to protect
accesses from lkey_table.  When a rvt_mregion needs to be freed, it
first gets unregistered from lkey_table and then rvt_check_refs() is
called to wait for in-flight usages before the rvt_mregion is freed.

rvt_check_refs() seems to have a couple issues.

* It has a fast exit path which tests percpu_ref_is_zero().  However,
  a percpu_ref reading zero doesn't mean that the object can be
  released.  In fact, the ->release() callback might not even have
  started executing yet.  Proceeding with freeing can lead to
  use-after-free.

* lkey_table is RCU protected but there is no RCU grace period in the
  free path.  percpu_ref uses RCU internally but it's sched-RCU whose
  grace periods are different from regular RCU.  Also, it generally
  isn't a good idea to depend on internal behaviors like this.

To address the above issues, this patch removes the the fast exit and
adds an explicit synchronize_rcu().

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: linux-rdma@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
Hello, Dennis, Mike.

I don't know RDMA at all and this patch is only compile tested.  Can
you please take a careful look?

Thanks.

 drivers/infiniband/sw/rdmavt/mr.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rdmavt/mr.c b/drivers/infiniband/sw/rdmavt/mr.c
index 1b2e536..cc429b5 100644
--- a/drivers/infiniband/sw/rdmavt/mr.c
+++ b/drivers/infiniband/sw/rdmavt/mr.c
@@ -489,11 +489,13 @@ static int rvt_check_refs(struct rvt_mregion *mr, const char *t)
 	unsigned long timeout;
 	struct rvt_dev_info *rdi = ib_to_rvt(mr->pd->device);
 
-	if (percpu_ref_is_zero(&mr->refcount))
-		return 0;
-	/* avoid dma mr */
-	if (mr->lkey)
+	if (mr->lkey) {
+		/* avoid dma mr */
 		rvt_dereg_clean_qps(mr);
+		/* @mr was indexed on rcu protected @lkey_table */
+		synchronize_rcu();
+	}
+
 	timeout = wait_for_completion_timeout(&mr->comp, 5 * HZ);
 	if (!timeout) {
 		rvt_pr_err(rdi,
-- 
2.9.5

  parent reply	other threads:[~2018-03-06 17:33 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-06 17:26 [PATCHSET] percpu_ref, RCU: Audit RCU usages in percpu_ref users Tejun Heo
2018-03-06 17:33 ` [PATCH 1/7] fs/aio: Add explicit RCU grace period when freeing kioctx Tejun Heo
2018-03-06 17:33   ` [PATCH 2/7] fs/aio: Use RCU accessors for kioctx_table->table[] Tejun Heo
2018-03-06 17:33   ` Tejun Heo [this message]
2018-03-07 15:39     ` [PATCH 3/7] RDMAVT: Fix synchronization around percpu_ref Dennis Dalessandro
2018-03-06 17:33   ` [PATCH 4/7] HMM: Remove superflous RCU protection around radix tree lookup Tejun Heo
2018-03-06 17:33     ` Tejun Heo
2018-03-06 17:59     ` Jerome Glisse
2018-03-06 17:59       ` Jerome Glisse
2018-03-06 17:33   ` [PATCH 5/7] block: Remove superflous rcu_read_[un]lock_sched() in blk_queue_enter() Tejun Heo
2018-03-06 17:52     ` Bart Van Assche
2018-03-14 18:46       ` tj
2018-03-14 20:05         ` Bart Van Assche
2018-03-14 20:08           ` Peter Zijlstra
2018-03-14 20:14             ` Bart Van Assche
2018-03-06 17:33   ` [PATCH 6/7] percpu_ref: Update doc to dissuade users from depending on internal RCU grace periods Tejun Heo
2018-03-06 17:33   ` [PATCH 7/7] RCU, workqueue: Implement rcu_work Tejun Heo
2018-03-06 18:30     ` Linus Torvalds
2018-03-09 15:37       ` Tejun Heo
2018-03-07  2:49     ` Lai Jiangshan
2018-03-07 14:54       ` Paul E. McKenney
2018-03-07 16:23         ` Peter Zijlstra
2018-03-07 17:58           ` Paul E. McKenney
2018-03-08  0:29         ` Lai Jiangshan
2018-03-08 17:28           ` Paul E. McKenney
2018-03-09 16:21           ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180306173316.3088458-3-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=bcrl@kvack.org \
    --cc=dennis.dalessandro@intel.com \
    --cc=jannh@google.com \
    --cc=kent.overstreet@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mike.marciniszyn@intel.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=security@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.