Re: [PATCH 3/7] RDMAVT: Fix synchronization around percpu_ref

From: Dennis Dalessandro
Date: Wed Mar 07 2018 - 10:40:12 EST


On 3/6/2018 12:33 PM, Tejun Heo wrote:
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

Typo above too many "the".

adds an explicit synchronize_rcu().

Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
Cc: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>
Cc: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
Cc: linux-rdma@xxxxxxxxxxxxxxx
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
Hello, Dennis, Mike.

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

Looks good to me, passes my basic sanity tests. Thanks!

Acked-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>