From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 03530C43460 for ; Fri, 14 May 2021 07:54:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BB41C61453 for ; Fri, 14 May 2021 07:54:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233306AbhENHzd (ORCPT ); Fri, 14 May 2021 03:55:33 -0400 Received: from szxga06-in.huawei.com ([45.249.212.32]:2979 "EHLO szxga06-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233293AbhENHzc (ORCPT ); Fri, 14 May 2021 03:55:32 -0400 Received: from DGGEMS408-HUB.china.huawei.com (unknown [172.30.72.59]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4FhLMq2v9wzmWHl; Fri, 14 May 2021 15:52:07 +0800 (CST) Received: from [10.174.179.215] (10.174.179.215) by DGGEMS408-HUB.china.huawei.com (10.3.19.208) with Microsoft SMTP Server id 14.3.498.0; Fri, 14 May 2021 15:54:18 +0800 Subject: Re: [PATCH] IB/srpt: Fix passing zero to 'PTR_ERR' To: Jason Gunthorpe , Bart Van Assche CC: , , , References: <20201112145443.17832-1-yuehaibing@huawei.com> <20201112172008.GA944848@nvidia.com> <20201112183023.GB917484@nvidia.com> From: YueHaibing Message-ID: <9afea945-8991-4c27-10d0-e02c732705fc@huawei.com> Date: Fri, 14 May 2021 15:54:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <20201112183023.GB917484@nvidia.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.179.215] X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: target-devel@vger.kernel.org On 2020/11/13 2:30, Jason Gunthorpe wrote: > On Thu, Nov 12, 2020 at 10:25:48AM -0800, Bart Van Assche wrote: >> On 11/12/20 9:20 AM, Jason Gunthorpe wrote: >>> I think it should be like this, Bart? >>> >>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> index 6017d525084a0c..80f9673956ced2 100644 >>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> @@ -2311,7 +2311,7 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, >>> mutex_lock(&sport->port_guid_id.mutex); >>> list_for_each_entry(stpg, &sport->port_guid_id.tpg_list, entry) { >>> - if (!IS_ERR_OR_NULL(ch->sess)) >>> + if (ch->sess) >>> break; >>> ch->sess = target_setup_session(&stpg->tpg, tag_num, >>> tag_size, TARGET_PROT_NORMAL, >>> @@ -2321,12 +2321,12 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, >>> mutex_lock(&sport->port_gid_id.mutex); >>> list_for_each_entry(stpg, &sport->port_gid_id.tpg_list, entry) { >>> - if (!IS_ERR_OR_NULL(ch->sess)) >>> + if (ch->sess) >>> break; >>> ch->sess = target_setup_session(&stpg->tpg, tag_num, >>> tag_size, TARGET_PROT_NORMAL, i_port_id, >>> ch, NULL); >>> - if (!IS_ERR_OR_NULL(ch->sess)) >>> + if (ch->sess) >>> break; >>> /* Retry without leading "0x" */ >>> ch->sess = target_setup_session(&stpg->tpg, tag_num, >>> @@ -2335,7 +2335,9 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, >>> } >>> mutex_unlock(&sport->port_gid_id.mutex); >>> - if (IS_ERR_OR_NULL(ch->sess)) { >>> + if (!ch->sess) >>> + ch->sess = ERR_PTR(-ENOENT); >>> + if (IS_ERR(ch->sess)) { >>> WARN_ON_ONCE(ch->sess == NULL); >>> ret = PTR_ERR(ch->sess); >>> ch->sess = NULL; >>> >> >> Hi Jason, >> >> The ib_srpt driver accepts three different formats for the initiator ACL. Up >> to two of the three target_setup_session() calls will fail if the fifth >> argument of target_setup_session() does not use the format of the initiator >> ID in configfs. If the first or the second target_setup_session() call fails >> it is essential that later target_setup_session() calls happen. Hence the >> IS_ERR_OR_NULL(ch->sess) checks in the above loops. > > IS_ERR_OR_NULL is an abomination, it should not be used. > > I see I didn't quite get it right, but that is still closer to sane, > probably target_setup_session() should return NULL not err_ptr Any fix plan? > > Jason > . >