Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] target: Fix a double put in transport_free_session
To:     Maurizio Lombardi <mlombard@redhat.com>,
        Lv Yunlong <lyl2019@mail.ustc.edu.cn>,
        martin.petersen@oracle.com
Cc:     linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20210323025851.11782-1-lyl2019@mail.ustc.edu.cn>
 <f03c8a67-d015-d503-726b-647b4f327b25@redhat.com>
From:   Mike Christie <michael.christie@oracle.com>
Message-ID: <0553e265-2795-86fa-5f8b-634d8060ec84@oracle.com>
Date:   Fri, 26 Mar 2021 11:24:16 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
In-Reply-To: <f03c8a67-d015-d503-726b-647b4f327b25@redhat.com>
Content-Type: text/plain; charset=iso-8859-2
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [20.15.0.204] (73.88.28.6) by DM5PR22CA0002.namprd22.prod.outlook.com (2603:10b6:3:101::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3977.25 via Frontend Transport; Fri, 26 Mar 2021 16:24:18 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 488792b7-7445-4d5e-e21b-08d8f0739b5d
X-MS-TrafficTypeDiagnostic: BYAPR10MB3574:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <BYAPR10MB3574E9E41C44C63309AE75B9F1619@BYAPR10MB3574.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:8882;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: 0cQSiBXqcB4af0r0bie2gZZ4NmBQA90baCn7UTFY8mltNm9oRxSM7DeN9VGvrXPxmEW2im7E1F21nbKE0Z21cnP8YLnJHDlywp/HOZK9YgErNeTOroDJMtiaxdgF4fY8d/vh8ShuAiOboQMykS/k12saD45D5NEX9SY3M3hooBp51enLeBRw6l3DuQzNEPEMxU3EarL4x109t8Wz9HfvTiVrb/QN5NXGS5CHx68R+B86cjORCyWHAX/rVj/uR0J+JjsCV0ve1EZ5gPVHiyXRVem8j2izU8E/9HafxzYKyEcbuTS/LOti1vPE3+h37nd0pbrJ0QWS17RUUl0oaXu0K7957h4wu1e/DMJbDUwHvxrat89I4gACqopMqpbwHKRb0U+I34oHOCgwgjdkcYuL6pBAQmIFwK4MUeWMK0/d8q+t8eGl/0Dnm2esQn5aatLVVNA6dw16mLMfjLlT8V4icWpPRAs1nAs2subLhrTYu+t6PU0DigfUE7hKwjKTJGNfEgOXe7sKCH8wPTyjBAtuEZSXkvG9N62z9jEPWPfd19msqPQb9DG4yRg9D6gvfxmy0BLepHyBGKBAmhXI4x5fbNdeRq8j2eAjpBaubfcPeLnWhDZF/UI92c+83N1gMbSd1OMpfaB0Q+n+TyEWCAr6tMOsfBK16tF0ZXMypHwJGRkj2/beYugDq4eKYQKaN2cxG/WNaWFSAtoaIoeo0AJxAtWp16kNYGmcOXrsKFgUuCAlC67MEMpZN/UGNLIJDFek
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR10MB3573.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(396003)(39860400002)(376002)(136003)(346002)(366004)(31696002)(66556008)(66476007)(31686004)(66946007)(6486002)(38100700001)(2906002)(4326008)(6706004)(53546011)(186003)(2616005)(6636002)(956004)(8936002)(86362001)(26005)(83380400001)(5660300002)(110136005)(8676002)(16526019)(36756003)(316002)(478600001)(16576012)(78286007)(43740500002)(45980500001);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: =?iso-8859-2?Q?pcSB9oclSHM3KBXGPZ6T0L1PBtrhJg2KDTR/9cL9kL1ZdfnwSHudDVA99t?=
 =?iso-8859-2?Q?U70wOsY0LEmN+yW9ZGoKUVB3lWh8TdzWY2rIthmK56pypIkjj53uhpJEpz?=
 =?iso-8859-2?Q?A/6jcyGxbawsJoJB9mMYtmOJTeXNcON458t9bMHxhYLxJR+Oo1WbhSkPXD?=
 =?iso-8859-2?Q?TLiJlVMg4oazh2pe5btbFd7m7wGZSUykv5/cAuRoGyWxklhvmbCl3B+sFh?=
 =?iso-8859-2?Q?TH0aWy/FjE23SX8dlVV3XQFPOcjj9vs4k9Qf5S/4Irrf8odo+NXQXKOODu?=
 =?iso-8859-2?Q?rHYNT9s4lDqdhmNnyxqdo9xyDUlYsiiDo6B8vJLweAJxfUUuVXfj4HYei+?=
 =?iso-8859-2?Q?BcKPzK+FH8kHUIhnyYXDB4xlYLNHEhyNwych8n52s00MrRUCKq/3EK3JT4?=
 =?iso-8859-2?Q?OWnwZzleDec6bvBSjWNwKtOqL8MR/YCmfvRf6Ec1KjHboMhPREdHt5t2NX?=
 =?iso-8859-2?Q?rCILxjt2XpKbp8134Hv89IyNwRfMqfO52ExEnMvpCXv03PYi0KbgVFOfNM?=
 =?iso-8859-2?Q?O4Y9GsinheQJ0QdeO3Df7MefWS6FA/vMJgr8D0WCoy339Xq8ujuRcqYItY?=
 =?iso-8859-2?Q?c+4HMi9dBCJu5zz7orhY1of9n+/i3SUmE9ZlkXJqrSzsLRB4/6NW5lCxK7?=
 =?iso-8859-2?Q?4sbYso/7nfCBwRBoQR81fsv/08r0Kjy5xyhT/WK7R4N40EpgzEzqUKDqbr?=
 =?iso-8859-2?Q?6rVaCJWcANUIHG+Tee2LZFY5oYpkw+jA0spNUBmvk7HJpkIp49Z3PLixBE?=
 =?iso-8859-2?Q?wkuoxuoR9qIcO2Kojn1EZlMEKJcElZR4RIcxIxkxee6eqmryZAMGN3joeP?=
 =?iso-8859-2?Q?m+fHNqbF7qkB4/2qB2r0FzoT342wyO0cni4jOMKP5E9C/ljYAmMlyTmmA2?=
 =?iso-8859-2?Q?M69fFhppREAuc9B2QEWA50NZvDI9qFrpbU31StEoKJX4zudoZyuc0zUOtg?=
 =?iso-8859-2?Q?Zd60JaPMKFH+81oXCkbuzXIe84zVahNT3Iqvz6Wi4DLuKKdnY+77zYYhri?=
 =?iso-8859-2?Q?O47X1fiCzB+LKu9WkxXeyrP8IED0krUXE259o9NnNoP5m2EBzv6kWabJHI?=
 =?iso-8859-2?Q?+5d0VdWv/3peIHqN6PZktjXtitNLIS4oSC7+obnI0+D+Uqf40ROBnOMUKb?=
 =?iso-8859-2?Q?zK9/GV3PpOkCVS20q0XSObUmydjUTQqweXRvvSx2inLQzKeC6aCqx+HgCp?=
 =?iso-8859-2?Q?n4x7pESh4W+xKFReHLi/g2LlT/9992Elx4dOu6lgjYglmTLKKiVODUVqnu?=
 =?iso-8859-2?Q?lFWoSIa7pWFr4pHFfBWTdDMn2n/fQaLqYxz4OAQcUpHxaJBuYcNjVPsDPc?=
 =?iso-8859-2?Q?/kXf5wshwp3PH4YGs/2RWCFoRUF4YIH7EDNzatDkHAf81gucuayNUa1gE0?=
 =?iso-8859-2?Q?iN1tSSZ/xN?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 488792b7-7445-4d5e-e21b-08d8f0739b5d
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB3573.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Mar 2021 16:24:19.5279
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fT80WwtGmMOsFoQ9LlCcrTv80KpcoSIaPQlzxCx+/BwW0HIn4sn2gStecVUn/o/4Skcrdw3mpShp/e1UfUbPU5HIvYfujWLcGwOYnpNmRqI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3574
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9935 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 bulkscore=0
 spamscore=0 mlxlogscore=999 suspectscore=0 adultscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2103250000
 definitions=main-2103260122
X-Proofpoint-ORIG-GUID: a_mLnJurz8iRRUbyoFR2C-fSsCn8DVBg
X-Proofpoint-GUID: a_mLnJurz8iRRUbyoFR2C-fSsCn8DVBg
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9935 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 lowpriorityscore=0 malwarescore=0
 suspectscore=0 adultscore=0 phishscore=0 mlxscore=0 mlxlogscore=999
 impostorscore=0 priorityscore=1501 clxscore=1015 spamscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2103250000
 definitions=main-2103260122
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/26/21 7:31 AM, Maurizio Lombardi wrote:
> 
> 
> Dne 23. 03. 21 v 3:58 Lv Yunlong napsal(a):
>> In transport_free_session, se_nacl is got from se_sess
>> with the initial reference. If se_nacl->acl_sess_list is
>> empty, se_nacl->dynamic_stop is set to true. Then the first
>> target_put_nacl(se_nacl) will drop the initial reference
>> and free se_nacl. Later there is a second target_put_nacl()
>> to put se_nacl. It may cause error in race.
>>
>> My patch sets se_nacl->dynamic_stop to false to avoid the
>> double put.
>>
>> Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn>
>> ---
>>  drivers/target/target_core_transport.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
>> index 5ecb9f18a53d..c266defe694f 100644
>> --- a/drivers/target/target_core_transport.c
>> +++ b/drivers/target/target_core_transport.c
>> @@ -584,8 +584,10 @@ void transport_free_session(struct se_session *se_sess)
>>  		}
>>  		mutex_unlock(&se_tpg->acl_node_mutex);
>>  
>> -		if (se_nacl->dynamic_stop)
>> +		if (se_nacl->dynamic_stop) {
>>  			target_put_nacl(se_nacl);
>> +			se_nacl->dynamic_stop = false;
>> +		}
>>  
>>  		target_put_nacl(se_nacl);
>>  	}
>>
> 
> FYI,
> 
> I have received a bug report against the 5.8 kernel about task hangs that seems to involve the nacl "dynamic_stop" code
> 
> Mar  4 16:49:44 gzboot kernel: [186322.177819] INFO: task targetcli:2359053 blocked for more than 120 seconds.
> Mar  4 16:49:44 gzboot kernel: [186322.178862]       Tainted: P           O      5.8.0-44-generic #50-Ubuntu
> Mar  4 16:49:44 gzboot kernel: [186322.179746] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> Mar  4 16:49:44 gzboot kernel: [186322.180583] targetcli       D    0 2359053 2359052 0x00000000
> Mar  4 16:49:44 gzboot kernel: [186322.180586] Call Trace:
> Mar  4 16:49:44 gzboot kernel: [186322.180592]  __schedule+0x212/0x5d0
> Mar  4 16:49:44 gzboot kernel: [186322.180595]  ? usleep_range+0x90/0x90
> Mar  4 16:49:44 gzboot kernel: [186322.180596]  schedule+0x55/0xc0
> Mar  4 16:49:44 gzboot kernel: [186322.180597]  schedule_timeout+0x10f/0x160
> Mar  4 16:49:44 gzboot kernel: [186322.180601]  ? evict+0x14c/0x1b0
> Mar  4 16:49:44 gzboot kernel: [186322.180602]  __wait_for_common+0xa8/0x150
> Mar  4 16:49:44 gzboot kernel: [186322.180603]  wait_for_completion+0x24/0x30
> Mar  4 16:49:44 gzboot kernel: [186322.180637]  core_tpg_del_initiator_node_acl+0x8e/0x120 [target_core_mod]

We would need more details. We can hit that normally when the target driver
waits for stuck IO to complete before calling transport_deregister_session.
I think if you hit the put bug, then we would have seen the refcount warning
in refcount.h fire before the hung task because we do an extra put.

Did the user switch to ACLs? Did you see my comment about it looks like there
is a bug if the user were to add an acl while dynamic was used for the same
initiatorname. In that case, we do not do the put to match the one taken
core_tpg_check_initiator_node_acl. In that case we would hit your hang since
no one ever does the last put.