From patchwork Fri Jul 31 00:23:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 6907621 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A30239F38B for ; Fri, 31 Jul 2015 00:23:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 21FBD204A2 for ; Fri, 31 Jul 2015 00:23:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CC14C204A0 for ; Fri, 31 Jul 2015 00:23:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751503AbbGaAXy (ORCPT ); Thu, 30 Jul 2015 20:23:54 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:33504 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751364AbbGaAXx (ORCPT ); Thu, 30 Jul 2015 20:23:53 -0400 Received: from [172.18.3.65] (unknown [157.22.22.146]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 8F54C22D9EB; Fri, 31 Jul 2015 00:19:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1438301948; bh=hWw0F1jO3luoliKTY6TRyfLVzmPvZTw TiQDaawb1ayM=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=jLSG/KA4wjBPLdX0D39B4Di7H89CwU0ZW/Dc7oqKx+8X6JtWX2axqUMtJlpPuO2YE Wx5w3UPK5dGEXRdp8XtqzJONLp8IkwW+QxChjp0Q3DCZQ+VQQUBlXFgq9IlYh8GDTL6 NIlIDwv9OhkOmG1rBHCl75QxMGKBdbua2LQvVa8= Message-ID: <1438302223.32325.7.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH] target: Wait RCU grace-period before backend/fabric unload From: "Nicholas A. Bellinger" To: paulmck@linux.vnet.ibm.com Cc: "Nicholas A. Bellinger" , target-devel , linux-scsi , Christoph Hellwig , Hannes Reinecke , Sagi Grimberg Date: Thu, 30 Jul 2015 17:23:43 -0700 In-Reply-To: <20150730130754.GB27280@linux.vnet.ibm.com> References: <1438236923-17889-1-git-send-email-nab@daterainc.com> <20150730130754.GB27280@linux.vnet.ibm.com> X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham 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, 2015-07-30 at 06:07 -0700, Paul E. McKenney wrote: > On Thu, Jul 30, 2015 at 06:15:23AM +0000, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > This patch addresses a v4.2-rc1 regression where backend driver > > struct module unload immediately after ->free_device() has done > > an internal call_rcu(), results in IRQ rcu_process_callbacks() > > use-after-free paging OOPsen. > > > > It adds a explicit synchronize_rcu() in target_backend_unregister() > > to wait a full RCU grace period before releasing target_backend_ops > > memory, and allowing TBO->module exit to proceed. > > Good catch, but... > > You need rcu_barrier() rather than synchronize_rcu() in this case. > All that synchronize_rcu() does is wait for pre-existing RCU readers, > when what is needed is to wait for all pre-existing RCU callbacks > to be invoked. > Ah, was getting confused by rcu_barrier_tasks() being specific to CONFIG_TASKS_RCU in update.c code, and missing rcu_barrier() in tree_plugin.h. Should have taken a look at Documentation/RCU/rcubarrier.txt.. > So please replace the two synchronize_rcu() calls with rcu_barrier(). > , below is the updated version. Thanks for the review! --nab From 9721910116f9883c7afded613ec88dc2de12339a Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 29 Jul 2015 22:27:13 -0700 Subject: [PATCH] target: Perform RCU callback barrier before backend/fabric unload This patch addresses a v4.2-rc1 regression where backend driver module unload happening immediately after TBO->free_device() does internal call_rcu(), will currently result in IRQ context rcu_process_callbacks() use-after-free paging OOPsen. It adds the missing rcu_barrier() in target_backend_unregister() to perform an explicit RCU barrier waiting for all RCU callbacks to complete before releasing target_backend_ops memory, and allowing TBO->module exit to proceed. Also, do the same for fabric drivers in target_unregister_template() to ensure se_deve_entry->rcu_head -> kfree_rcu() callbacks have completed, before allowing target_core_fabric_ops->owner module exit to proceed. Cc: Paul E. McKenney Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Sagi Grimberg Signed-off-by: Nicholas Bellinger Acked-by: Paul E. McKenney --- drivers/target/target_core_configfs.c | 9 ++++++++- drivers/target/target_core_hba.c | 10 +++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index c2e9fea..860e840 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -457,8 +457,15 @@ void target_unregister_template(const struct target_core_fabric_ops *fo) if (!strcmp(t->tf_ops->name, fo->name)) { BUG_ON(atomic_read(&t->tf_access_cnt)); list_del(&t->tf_list); + mutex_unlock(&g_tf_lock); + /* + * Wait for any outstanding fabric se_deve_entry->rcu_head + * callbacks to complete post kfree_rcu(), before allowing + * fabric driver unload of TFO->module to proceed. + */ + rcu_barrier(); kfree(t); - break; + return; } } mutex_unlock(&g_tf_lock); diff --git a/drivers/target/target_core_hba.c b/drivers/target/target_core_hba.c index 62ea4e8..be9cefc 100644 --- a/drivers/target/target_core_hba.c +++ b/drivers/target/target_core_hba.c @@ -84,8 +84,16 @@ void target_backend_unregister(const struct target_backend_ops *ops) list_for_each_entry(tb, &backend_list, list) { if (tb->ops == ops) { list_del(&tb->list); + mutex_unlock(&backend_mutex); + /* + * Wait for any outstanding backend driver ->rcu_head + * callbacks to complete post TBO->free_device() -> + * call_rcu(), before allowing backend driver module + * unload of target_backend_ops->owner to proceed. + */ + rcu_barrier(); kfree(tb); - break; + return; } } mutex_unlock(&backend_mutex);