From patchwork Fri Oct 21 08:19:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 9388209 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 D1A4C607F0 for ; Fri, 21 Oct 2016 08:19:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C3B7C29EDE for ; Fri, 21 Oct 2016 08:19:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B7BDF29EE0; Fri, 21 Oct 2016 08:19:31 +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=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham 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 47D3629EDE for ; Fri, 21 Oct 2016 08:19:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754386AbcJUIT1 (ORCPT ); Fri, 21 Oct 2016 04:19:27 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:47029 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754310AbcJUITY (ORCPT ); Fri, 21 Oct 2016 04:19:24 -0400 Received: from [192.168.1.66] (75-37-194-224.lightspeed.lsatca.sbcglobal.net [75.37.194.224]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id ECB2740B05; Fri, 21 Oct 2016 08:19:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1477037970; bh=3QMv1TEdzTiy/7hk08p7RPlHvm004Mh 7HuGM1o7boV0=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=UdvrEfw8zVkAFrEfNY39EUYTSfBple578nfAwTeXR2aIq09PQw/tlsbdVCCpZiCaC YN70J3r7ySmYsMI1Uqw+hQxrsqAE3ZXd0paQe8BR+dhMxBGQDV5EU2IfSZYfHCj6I3P FXUM4Zv0xkEPG3MzkbSENZqjkEgUqfyEXHnsMOk= Message-ID: <1477037962.8490.163.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH 2/6] target: fix potential race window in target_sess_cmd_list_waiting() From: "Nicholas A. Bellinger" To: Hannes Reinecke Cc: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org, Martin Wilck , Hannes Reinecke Date: Fri, 21 Oct 2016 01:19:22 -0700 In-Reply-To: <1471856051-87398-3-git-send-email-hare@suse.de> References: <1471856051-87398-1-git-send-email-hare@suse.de> <1471856051-87398-3-git-send-email-hare@suse.de> 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-Virus-Scanned: ClamAV using ClamSMTP On Mon, 2016-08-22 at 10:54 +0200, Hannes Reinecke wrote: > target_sess_cmd_list_waiting() might hit on a condition where > the kref for the command is already 0, but the destructor has > not been called yet (or is stuck in waiting for a spin lock). > Rather than leaving the command on the list we should explicitly > remove it to avoid race issues later on. > > Signed-off-by: Hannes Reinecke > --- > drivers/target/target_core_transport.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > index 2e1a6d8..ce136f0 100644 > --- a/drivers/target/target_core_transport.c > +++ b/drivers/target/target_core_transport.c > @@ -2547,8 +2547,8 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref) > * fabric acknowledgement that requires two target_put_sess_cmd() > * invocations before se_cmd descriptor release. > */ > - if (ack_kref) > - kref_get(&se_cmd->cmd_kref); > + if (ack_kref && !kref_get_unless_zero(&se_cmd->cmd_kref)) > + return -EINVAL; > Makes sense. Applying the following version to target-pending/master atop pending SCF_ACK_KREF regression bug-fix. > spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); > if (se_sess->sess_tearing_down) { > @@ -2627,7 +2627,7 @@ EXPORT_SYMBOL(target_put_sess_cmd); > */ > void target_sess_cmd_list_set_waiting(struct se_session *se_sess) > { > - struct se_cmd *se_cmd; > + struct se_cmd *se_cmd, *tmp_cmd; > unsigned long flags; > int rc; > > @@ -2639,7 +2639,8 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess) > se_sess->sess_tearing_down = 1; > list_splice_init(&se_sess->sess_cmd_list, &se_sess->sess_wait_list); > > - list_for_each_entry(se_cmd, &se_sess->sess_wait_list, se_cmd_list) { > + list_for_each_entry_safe(se_cmd, tmp_cmd, > + &se_sess->sess_wait_list, se_cmd_list) { > rc = kref_get_unless_zero(&se_cmd->cmd_kref); > if (rc) { > spin_lock(&se_cmd->t_state_lock); > @@ -2648,7 +2649,8 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess) > se_cmd->transport_state |= CMD_T_FABRIC_STOP; > } > spin_unlock(&se_cmd->t_state_lock); > - } > + } else > + list_del_init(&se_cmd->se_cmd_list); > } > > spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); --- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 9ebbf94..ad6fb3f 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2535,7 +2535,9 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref) * invocations before se_cmd descriptor release. */ if (ack_kref) { - kref_get(&se_cmd->cmd_kref); + if (!kref_get_unless_zero(&se_cmd->cmd_kref)) + return -EINVAL; + se_cmd->se_cmd_flags |= SCF_ACK_KREF; }