linux-kernel - Re: [PATCH 2/2] iscsi: set session to FREE state after unbind session in remove session

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Apr 2022 10:30:40 -0500
From:   Mike Christie <michael.christie@...cle.com>
To:     Wenchao Hao <haowenchao@...wei.com>, Lee Duncan <lduncan@...e.com>,
        Chris Leech <cleech@...hat.com>,
        "James E . J . Bottomley" <jejb@...ux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        open-iscsi@...glegroups.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     linfeilong@...wei.com
Subject: Re: [PATCH 2/2] iscsi: set session to FREE state after unbind session
 in remove session

On 4/13/22 8:49 PM, Wenchao Hao wrote:
> __iscsi_unbind_session() set session state to ISCSI_SESSION_UNBOUND, which
> would overwrite the ISCSI_SESSION_FREE state.
> 
> Signed-off-by: Wenchao Hao <haowenchao@...wei.com>
> ---
>  drivers/scsi/scsi_transport_iscsi.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index 97a9fee02efa..d8dd9279cea8 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2173,6 +2173,22 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
>  	if (!cancel_work_sync(&session->block_work))
>  		cancel_delayed_work_sync(&session->recovery_work);
>  	cancel_work_sync(&session->unblock_work);
> +
> +	scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
> +	/*
> +	 * qla4xxx can perform it's own scans when it runs in kernel only
> +	 * mode. Make sure to flush those scans.
> +	 */
> +	flush_work(&session->scan_work);
> +
> +	/*
> +	 * flush running unbind operations
> +	 * if unbind work did not queued, call __iscsi_unbind_session
> +	 * directly to perform target remove

We probably don't need the flush_work test because we are going to
normally call __iscsi_unbind_session.

If the unbind work had already run, which is the normal case, then
flush_work returns false and we end up calling __iscsi_unbind_session
like before. That function then checks if the target is really unbound.
So the extra check doesn't normally buy us anything with your patches
because in patch 1 you fixed it so __iscsi_unbind_session doesn't send
the extra event.


> +	 */
> +	if (!flush_work(&session->unbind_work))
> +		__iscsi_unbind_session(&session->unbind_work);
> +
>  	/*
>  	 * If we are blocked let commands flow again. The lld or iscsi
>  	 * layer should set up the queuecommand to fail commands.
> @@ -2183,16 +2199,6 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
>  	session->state = ISCSI_SESSION_FREE;
>  	spin_unlock_irqrestore(&session->lock, flags);
>  
> -	scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
> -	/*
> -	 * qla4xxx can perform it's own scans when it runs in kernel only
> -	 * mode. Make sure to flush those scans.
> -	 */
> -	flush_work(&session->scan_work);
> -	/* flush running unbind operations */
> -	flush_work(&session->unbind_work);
> -	__iscsi_unbind_session(&session->unbind_work);
> -
>  	/* hw iscsi may not have removed all connections from session */
>  	err = device_for_each_child(&session->dev, NULL,
>  				    iscsi_iter_destroy_conn_fn);