Received-SPF: pass (google.com: domain of linux-kernel+bounces-108068-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <6bc61553-6c8e-4705-9cbb-8e73d3f8c801@oracle.com>
Date: Tue, 19 Mar 2024 14:33:56 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] iscsi_tcp: disallow binding the same connection twice
Content-Language: en-US
To: Khazhismel Kumykov <khazhy@chromium.org>, 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>
Cc: open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org, Khazhismel Kumykov <khazhy@google.com>
References: <20240318194902.3290795-1-khazhy@google.com>
 <20240318194902.3290795-2-khazhy@google.com>
From: Mike Christie <michael.christie@oracle.com>
In-Reply-To: <20240318194902.3290795-2-khazhy@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?dHV3MnJQbk5zOHd4NERpUFBtdTRnQ0NYTzJBQlYweTBrMjJGYnJzbWtWd0Nn?=
 =?utf-8?B?NGwzUmJLRDdsQ3NjQ0ZieWIrTEpaNkcvN2c0OFVjTmdCN1dwNlFicnl0Z3lZ?=
 =?utf-8?B?TWRGNFpGQ3RMdjQyT2JqMC9VcCtxYzdIeEYxV2xYa0pQWmhPRlZUTitDVDha?=
 =?utf-8?B?R0ZmckNDZ2NzMEI2SGhrU1BKRnVKRHA2RVBqcjc2SW56QnZhS0ZxMXhjNFUw?=
 =?utf-8?B?NVZXQzFvT1pNWGhmQ2NWOGd4VTVIaDB3UlVYSmowWDJXakU2MFdkQ2FkQUcz?=
 =?utf-8?B?SW1zQktZVzhyVDRKNkpzMlRPTGw5MU9pc2s0eXJDV2paaGJNODZ5eldFd0xL?=
 =?utf-8?B?cjFXL2ZTdHhCeWVld1V0aTEyUnVmTm5iRjNudGF4YTYrZTExM3oyc0plWHRa?=
 =?utf-8?B?U3gzZmRKTjhOWE5rVUJhLzU4VndaalVQWE9IYUtBeXdBcTRpMDFFNzBKaEU0?=
 =?utf-8?B?bFdyOVI0dXZZSlpiVm56YnFhYVlrUFlXYXdoS3RVSEc4VFFvSitaOG02OGhl?=
 =?utf-8?B?TnJGMFl3eVZZamhDYzNyQ0E3SHlRZXNYek5yc2xrek1YODgrbGFvL29wMTRr?=
 =?utf-8?B?TjVrLzZ6a2RwSW9ydHVUaWNFeGdvNGpwQklnUTB0enlFakNnYXZ5dExDMi9v?=
 =?utf-8?B?UnNBYzF1UC91eXNQNkxwaGJhbjlwU3Ztb3ZFRXh6RytHaWgwSGQ0UEhVKzRM?=
 =?utf-8?B?RmVnYUtVOTZUeVVHOVJ6OHRQdzVZbmczY3ZDUXFtSU5hY1RSbTkyLzE1WTJk?=
 =?utf-8?B?VEFMSm1meTBBcmZ3REZGUTM0a285L2JKSW03QlVYdWljNnJubUlPS3dEbTBi?=
 =?utf-8?B?R3piTWRkR2NybHM5TG1qMW1vVU1nSElLQ1lKZzExakhZZlpGZ2hWMVZoTWph?=
 =?utf-8?B?ZUN0QzVTY3V4OWpQeVZjOUFUNmJpWUc4OUFQSmwxTDFMSENsMlF4cmRNUnhq?=
 =?utf-8?B?ZDNHdW5udVB4bXdBVVhONUtCMXAxbWxHYVd6R212V3pFZTdPcXU3WmRQUStj?=
 =?utf-8?B?S1Y2ZUM4YTZ5Nmp4bVFjSHFqb2JTRjVQelQ4MkYvelZBMkpOekZmOGNJcTZV?=
 =?utf-8?B?YVpVWGtBdW80T0pWeGpkcmcrd29VQi9VL0JEYVdhbkJXZTg1Z2tZSVgzcVB3?=
 =?utf-8?B?VjdBTkhOTjZSR1k0MkpFZktJSGNwV2lMS2t2c3hBdVY2OEpzQklvbDUvbXpD?=
 =?utf-8?B?blJYNE1FNjJQbjU2Vk1JY0pNS1kvVmJWenR5VUNHTFpqb1kwRXpHdVpyaEpY?=
 =?utf-8?B?czRmNXptSkVOUEJpK1dBZldZVGJ2S1JOdUlyV1R3TjJyU2ZrWUxBOHkrSmlW?=
 =?utf-8?B?ZHFobWE5SkdSZ2RDRlFmRnlLUDQxTjEyeHdza29zbXB5dFJNZGx1TWc3eGc4?=
 =?utf-8?B?citkdTZpL2R4cUFpV3EvLzJpNG8zT21CV3VodHJFVVVYVzBYeGN5bFhMNzJl?=
 =?utf-8?B?b250WjR1aDFlcmdiV201b0FzSWlKeDJvdk43M3g1WEljWkVuUXAySFlCM05p?=
 =?utf-8?B?cFlnVHpLcmdFenQyblV4LzI0SDhVaEdzMnpLejlWS0ZpRHBwMkVxM25HRUY3?=
 =?utf-8?B?dW85eW00eHVMa2dFMEIwejBKbXVBeWRwV2RldDNKS2pOSWorNDV3SmE1b2to?=
 =?utf-8?B?d1BhL0VnbW9ZSTVNTnFxMnRCQzIwNjVzOGxBcU0zTHRTQ1lvZmZZclMycnFh?=
 =?utf-8?B?Um9lZGJNZ1UwS3M2bEJPbEJwZzYxdDVHVitnUXIrcjhyMnlYaE0vOFlSRVpK?=
 =?utf-8?B?ZVZLdDFXYVFkZE9rSXVtN1FjMXR1Z3JXZk0rM3R5TlBLMFNUSFIvQ0dTRFI0?=
 =?utf-8?B?aG82VWNUdVgrMFRTbWQ3ZnJHczJudUt4WDNWVytmOEtUZ1kvaUhFSXk1S21P?=
 =?utf-8?B?Sk92TkR2dmhHNjkxcnY5RElUMTJ5QlpjUUtFblZzL3ZwanVNbU05YTREZmhS?=
 =?utf-8?B?Q0pXNTc4MVhickRBRnErZ1JEVFlteHdvb3M0VHlGTUtrRXpJbXVYSDNEY3hh?=
 =?utf-8?B?UmZwSkFPckVadWtoZUErRVliNjV6bzZkYmZIYW9rR2RJVkYwNWhaL0VnZFB6?=
 =?utf-8?B?MFpzZHpIVWpmYmJ1cmFIYTlWYU94NDRvL0g3MkY1Z3NGSm1LOW1FU08zalZr?=
 =?utf-8?B?TDM4cUZtakdvL2dtZWkzL1J4QUNKcnBodmdac1hIanJSSGJMR2RIaURPQng2?=
 =?utf-8?B?R0E9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	M5O6R6YPJk5UmQqcecLgheehCVBd9vSm8eQ1JzYdSAmTFL+XZg+GfysUQRo3X0q5j61G29VE2beU41sznaw+BYQauWOAsqCIKAZnopklMrdtpmGfgidvp/bwUi47XeHhQB+DQ4cOQaH+H+dnTMf4t1hP+8BrXAu3PRqP8B9PNCa1BVgdUcP21eAsmyCIuERRxld1jm3dUsXuP1tW0SD4o0JF6+UjcArxwCl2EZKtV5BxKMtom0GUfxTpMhZ55Qv/+CpAHu0Cklzg+EJnE874EAb3HaaBp/1duwYCeD0HT24Gr1URUinh41481MYTEsJGwryo/o2b0pMjKHbmBQ8VCUsPOT8i6eIWLsHA+2j1SrdIB/ohU5oJrVeeVbdWWq08Crk/IVpdXXKdtis1e/K5OaZ5F7e/FjbBF2rkj/M6zXsCm8Z49hsP2LB6WbYAYpFk+21JLxznOnSLD+KxmNINf4WpSngXZtFO9Mp/BplVdZLvsvKCjJdnvu3qjMALUH/7L6yRPQ02jnVvIkmXijsZIOivygdqlA/4LZ20vL9hzIiZU/YJZNdhW6/nxyuVe6cf6+AUGUzMuDgBUSWy5mjeM3Hw+06iAwk1xDGP9HAqTk0=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cc19fa6e-455d-4902-eadf-08dc484b85a2
X-MS-Exchange-CrossTenant-AuthSource: CY8PR10MB7243.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Mar 2024 19:33:58.2753
 (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: ij4FGLA1mitHFUBy4jgmttMA8sHqcIZOMAuFSFZPpcsCSLAWuxWDek2RpS+tawAXfmufDoy6ATazIFt2zLROY1a8SVfy/6Z3uTgt1ER5AIE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6966
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-19_08,2024-03-18_03,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=940
 malwarescore=0 adultscore=0 suspectscore=0 mlxscore=0 phishscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2403140000 definitions=main-2403190150
X-Proofpoint-GUID: yF5cwRY2V0_agWY60KmK2bKz6FRFQnKu
X-Proofpoint-ORIG-GUID: yF5cwRY2V0_agWY60KmK2bKz6FRFQnKu

On 3/18/24 2:49 PM, Khazhismel Kumykov wrote:
> iscsi_sw_tcp_conn_bind does not check or cleanup previously bound
> sockets, nor should we allow binding the same connection twice.
> 

This looks like a problem for all the iscsi drivers.

I think you could:

1. Add a check for ISCSI_CONN_FLAG_BOUND in iscsi_conn_bind.
2. Have iscsi_sw_tcp_conn_stop do:

        /* stop xmit side */
-       iscsi_suspend_tx(conn);
+       iscsi_conn_unbind(cls_conn, true);

to clear the flag when we clean up the conn for relogin.

3. Fix up the other iscsi drivers so they call:

iscsi_conn_unbind(cls_conn, true);

in their failure paths so when they fail they clear ISCSI_CONN_FLAG_BOUND and
iscsi_conn_bind can be called on the retry.


> Signed-off-by: Khazhismel Kumykov <khazhy@google.com>
> ---
>  drivers/scsi/iscsi_tcp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index e8ed60b777c6..8cf5dc203a82 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -716,6 +716,9 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
>  	struct socket *sock;
>  	int err;
>  
> +	if (tcp_sw_conn->sock)
> +		return -EINVAL;
> +
>  	/* lookup for existing socket */
>  	sock = sockfd_lookup((int)transport_eph, &err);
>  	if (!sock) {