Re: [PATCH AUTOSEL 6.18-5.10] scsi: ibmvfc: check for NULL evt in implicit LOGO and target delete path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: ibmvfc: invalid state transition in ibmvfc_tgt_implicit_logout_and_del()
--

commit 99489de7e63442e984a1136b57de120f99e8b4e7
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

scsi: ibmvfc: check for NULL evt in implicit LOGO and target delete path

This commit adds a NULL check for the event returned by
__ibmvfc_tgt_get_implicit_logout_evt() in ibmvfc_tgt_implicit_logout_and_del()
before passing it to ibmvfc_send_event().

Link: https://patch.msgid.link/20260723000149.969416-4-tyreld@linux.ibm.com

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 228daffb286de..66fe30b04b6a5 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -4403,6 +4403,14 @@ static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *tgt)
>  	evt = __ibmvfc_tgt_get_implicit_logout_evt(tgt,
>  						   ibmvfc_tgt_implicit_logout_and_del_done);
>  
> +	if (!evt) {
> +		vhost->discovery_threads--;
> +		ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);

[Severity: Medium]
Should this pass IBMVFC_TGT_ACTION_DEL_RPORT instead of
IBMVFC_TGT_ACTION_NONE?

ibmvfc_tgt_implicit_logout_and_del() is installed as tgt->job_step in
ibmvfc_del_tgt() when tgt->action is set to IBMVFC_TGT_ACTION_LOGOUT_RPORT:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_del_tgt() {
	if (!ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_LOGOUT_RPORT)) {
		tgt->job_step = ibmvfc_tgt_implicit_logout_and_del;
		tgt->init_retries = 0;
	}
}

and ibmvfc_do_work() only invokes it while tgt->action is
IBMVFC_TGT_ACTION_LOGOUT_RPORT:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_do_work() {
	case IBMVFC_HOST_ACTION_TGT_DEL:
	case IBMVFC_HOST_ACTION_TGT_DEL_FAILED:
		list_for_each_entry(tgt, &vhost->targets, queue) {
			if (tgt->action == IBMVFC_TGT_ACTION_LOGOUT_RPORT) {
				tgt->job_step(tgt);
				break;
			}
		}

		if (ibmvfc_dev_logo_to_do(vhost)) {
			spin_unlock_irqrestore(vhost->host->host_lock, flags);
			return;
		}
}

However, in ibmvfc_set_tgt_action(), when tgt->action is
IBMVFC_TGT_ACTION_LOGOUT_RPORT, only transitions to
IBMVFC_TGT_ACTION_LOGOUT_RPORT_WAIT or IBMVFC_TGT_ACTION_DEL_RPORT are
permitted:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_set_tgt_action() {
	switch (tgt->action) {
	case IBMVFC_TGT_ACTION_LOGOUT_RPORT:
		if (action == IBMVFC_TGT_ACTION_LOGOUT_RPORT_WAIT ||
		    action == IBMVFC_TGT_ACTION_DEL_RPORT) {
			tgt->action = action;
			rc = 0;
		}
		break;
}

Passing IBMVFC_TGT_ACTION_NONE here returns -EINVAL without changing
tgt->action, leaving the target in IBMVFC_TGT_ACTION_LOGOUT_RPORT instead
of transitioning it to IBMVFC_TGT_ACTION_DEL_RPORT like the other error
paths in ibmvfc_tgt_implicit_logout_and_del(). As a result,
ibmvfc_dev_logo_to_do(vhost) continues to return 1 and blocks rport
deletion in IBMVFC_HOST_ACTION_TGT_DEL / IBMVFC_HOST_ACTION_TGT_DEL_FAILED
(including during adapter removal when vhost->state is IBMVFC_HOST_OFFLINE).

> +		kref_put(&tgt->kref, ibmvfc_release_tgt);
> +		__ibmvfc_reset_host(vhost);
> +		return;
> +	}
> +
>  	ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_LOGOUT_RPORT_WAIT);
>  	if (ibmvfc_send_event(evt, vhost, default_timeout)) {
>  		vhost->discovery_threads--;

I hope this helps!
 Let me know if you need any further details.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-412-sashal@kernel.org?part=1