Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <823b30ce-c95e-4fc4-e634-f49c4433bcc9@oracle.com>
Date:   Wed, 9 Mar 2022 13:44:13 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [PATCH v3 0/3] scsi:iscsi: handle iscsi_cls_conn device with
 sysfs correctly
Content-Language: en-US
To:     Wenchao Hao <haowenchao@huawei.com>, Lee Duncan <lduncan@suse.com>,
        Chris Leech <cleech@redhat.com>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Wu Bo <wubo40@huawei.com>, Zhiqiang Liu <liuzhiqiang26@huawei.com>,
        linfeilong@huawei.com
References: <20220310015759.3296841-1-haowenchao@huawei.com>
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <20220310015759.3296841-1-haowenchao@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Rnh1RkdUMVNOdGMwVldsOFpMNE5aVld4K09wc3NyL1lBVjlvYVZSemY2S1FS?=
 =?utf-8?B?OUl3dTl5SHBQU3h1Um85d29qNS9mTkFpNncvYnR2cFVpY3lmZTB0RUR0Mksw?=
 =?utf-8?B?eXNKNkdoUWxrMDBxSFB4N0xjTXRqNkFBaFZ5RFRjZTNtcVcyeVB1SVlza01C?=
 =?utf-8?B?SFFqUVNIR1pHQkV3bWF3Um5HZ0ZlOEgyQ3J4U2lPUjVMWGIyUk5NYkhKY21J?=
 =?utf-8?B?MzBBRlVjZFdQdWM2U083QjFaN3M2ZkNPQmVzUXArWGhwbjBITUgzL29tbFBT?=
 =?utf-8?B?c0pkRlZYMGxPTFNCMWNjdUZyS3kxdFByYTY1UTd0U2t0S0FzQ1B2QndObTd1?=
 =?utf-8?B?T3Y3cHZTODVRVWNzU1NTdnRqeGxpSXdCRkhIc2MwZk5lZ29CQTBJUHEzcWZY?=
 =?utf-8?B?UktJUHZqVEgwWXV4UVhTWGNTdGg2WjJSbW5pMFRrQkFpVUZnaWwrd002Mkdn?=
 =?utf-8?B?bW96UitjNHEvVzdoUk9Wc0pKeElGT1lpWlNzS0lyQ1hRcDJYQ2VQbE1YY2ht?=
 =?utf-8?B?V1VCK3FWWmpMUUwzNVM2Y2hCRDVEOEUzVkRGSEovd3VhRHRYaXQwVXRkR0c3?=
 =?utf-8?B?K3Y5K3NHVGQzWm5vZnNvZTJLNnAzYUtsUTUvV29mWWdSTGNDaFhEd0hwSGta?=
 =?utf-8?B?alZwb1hXMHUwMW5Scyt5T2daYys1M01mZmNGT2k0SVM4SmpIaU5QRGJlL3c3?=
 =?utf-8?B?bTdMOFhwTWtPa1paYlZJZmprdWNaTFZVbHdLWDB2QkJjZFJ1OS9UR0t0WExD?=
 =?utf-8?B?U2pGQWlDa0ovSGcxQ1V1NU1HUE10dERROXBCSCtQUys5bENJQnlFMDdUYUEy?=
 =?utf-8?B?VFdpTlNIV0JQa0JUcWVBaFBOeTRaeUF6Q3pORXhzZVlYUU1zWHNEQmJBU0Np?=
 =?utf-8?B?OG5NUTJwVmt3ZE4yTDZTdHBZUWdxR1IxQjZoaEY3ZE13MXFPQ0lOcU1Wb25N?=
 =?utf-8?B?YldqbHpXTTJ6eW5aNTVtQVRaVXgyNVRzSG0wUHVUNm42SFlHblVUcTBPeDla?=
 =?utf-8?B?WGJQS3QvbEVHUkh0YmpUbklmakNiZFFFdWhXK0pBaW13d2MrbWF0eGd6OC9N?=
 =?utf-8?B?ZWtvZ1c3cmxKYlpDOFdOYUNUQVFIcUp5c2tta2QwZTBkTnlldFkwQzVvbHdM?=
 =?utf-8?B?NTNuWDFNMWd6N1J4Tk9NL2xLdGhRVjMwa3lZd3F0YjRraXFPSG9nNWNOZjlT?=
 =?utf-8?B?enB6bWh3VXhlcFpBVllTMnNNbHBSOXRkT0RUaVk2YVNmZkpXTGJBbVQvUElC?=
 =?utf-8?B?OG93SXcvWDhZeE1tdVFpRnZEd3F6b2VtZWl5cGNZcmUxdmtjOVJxWGd5VjdZ?=
 =?utf-8?B?em16dXF0a01WSDR3WDBueTA4aUZZT2hIeGxaSlkyS1A3VGhvdmRVVHBDclJi?=
 =?utf-8?B?eFprUEo5MkxkamJycVhuRUIzaTJadFlKd0tFbkxYcUovcnhvQzBYRW5KNFZt?=
 =?utf-8?B?aXJDRzFtT2lrM2NMOXgxNld1Q2NoM2srSGNyUkM1dUxVbXVrVHY4R20wLzg5?=
 =?utf-8?B?U0owbGYxUy9WSW1wSEFHOVVoQ3N4VXljZTl1V29ZdFF0b1Bna1BjZnBaSWN6?=
 =?utf-8?B?ZGQ2dzZCWWQvSUp3TUJUc0FYQldNMTNDc3VLbDRkRExLeU5ocWJITTd2bXJO?=
 =?utf-8?B?Ty9nUGVrelh1eEtxd0c4Q1RPMFJGaXdIeU9VK05FY0lWblZMSkJjc2tIWTJv?=
 =?utf-8?B?K3B0dDdOM1VUNDRSSzR0bm5jTXNlNlBwQWd5b3FrNXozZGIrYTZpUVBsd2tp?=
 =?utf-8?B?Y3MrcHNZN0RhV3ZaTjdUdUNYMFV3dDRic0Q0aDUyTkw0ZGtjak9QOHdrRnd2?=
 =?utf-8?B?V2pyWEk1bm1DMEo5Sm1zODZuR056Rkpydm53RG83Vk1sazBHZW5GOHgrTVND?=
 =?utf-8?B?d0lJSFZMZ0hJTmUxNk9iZWpRZnpjSG5VK2J0Qm9Kd0p3VWU4ZTVEL1ZDdWtN?=
 =?utf-8?B?NnNsNGpSaC83dXVwYzVLQURLWHN5RWk0WDEzbkR2d1V4S2dYaUtMcW1saWRr?=
 =?utf-8?B?MlBJU0U2WTVSSlczYVRGcXVnbG5hQWpoc1J5UUY0eUJnd21qKy9JZmhRSHNU?=
 =?utf-8?B?a0ZSUmlrNEowd3F0d0tab3ZlY2FkeUVpb0FodTlPZzRQQWIvYWVSWVBTcm1I?=
 =?utf-8?B?VC9GWVNhYUMvakV5MVdIeEpOMnprbzI5cDh2MzM3c1BlL1dXYkhrNEc3ZFZu?=
 =?utf-8?B?c04yZFZwcXJyR2VxQ3ZkaitnYUdHVHdsSmFrc3BMNEpud0wvV0dtbFljdk43?=
 =?utf-8?B?d3hXQ2ZpMmZ4d05nR2VSOFJzMTlnPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bc6b8ba1-1fe0-4444-ab1f-08da0205314f
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2022 19:44:15.2993
 (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: ERdhnNtlz8JQV92yG6gH2H56Psnpupt3bUJfEzbP2CChROlaw/tBAXyt1S4qN+ZF3L0gDYAQNOAtZ/ugmymixEieiDQvwir2wwQBvFa2cyo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4986
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10281 signatures=692062
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 phishscore=0 mlxscore=0
 bulkscore=0 mlxlogscore=999 spamscore=0 adultscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2203090104
X-Proofpoint-ORIG-GUID: T49dovCJ4EFS3ebarIk4qbYZugSHhwF-
X-Proofpoint-GUID: T49dovCJ4EFS3ebarIk4qbYZugSHhwF-
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/9/22 7:57 PM, Wenchao Hao wrote:
> We found a NULL pointer dereference in iscsi_sw_tcp_conn_get_param(),
> the root reason is we did sysfs addition wrong.
> 
> The origin implement do device setup in iscsi_create_conn() which
> bind the alloc/init and add in one function; do device teardown in
> iscsi_destroy_conn() which bind remove and free in one function.
> 
> This implement makes it impossible to initialize resources of device
> before add it to sysfs during setup.
> 
> So this patchset splict both the setup and teradown of iscsi_cls_conn to
> 2 steps.
> 
> For setup flow, we should call iscsi_alloc_conn() and initialize some
> resources, then call iscsi_add_conn().
> 
> For teradown flow, we should call iscsi_remove_conn() to remove device
> and free resources which related to iscsi_cls_conn, then call
> iscsi_put_conn() to free iscsi_cls_conn.
> 
> V2 -> V3:
>   * Fix some bugs and optimization the code implement.
> 
> V1 -> V2:
>   * add two more iscsi_free_conn() and iscsi_remove_conn() than V1
>   * change the teardown flow of iscsi_cls_conn
> 
> Wenchao Hao (3):
>   scsi: iscsi: Add helper functions to manage iscsi_cls_conn
>   scsi:libiscsi: Add iscsi_cls_conn to sysfs after been initialized
>   scsi:libiscsi: teradown iscsi_cls_conn gracefully
> 
>  drivers/scsi/libiscsi.c             | 23 +++++---
>  drivers/scsi/scsi_transport_iscsi.c | 90 +++++++++++++++--------------
>  include/scsi/scsi_transport_iscsi.h |  5 +-
>  3 files changed, 66 insertions(+), 52 deletions(-)
> 

Nice. Thanks.

Reviewed-by: Mike Christie <michael.christie@oracle.com>