Re: [PATCH v2 03/33] 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:
- [High] The error path added for a NULL event attempts an invalid state transition to IBMVFC_TGT_ACTION_NONE, leaving the target stuck in IBMVFC_TGT_ACTION_LOGOUT_RPORT and potentially causing an infinite host reset loop.
--

--- Patch [3]: [PATCH v2 03/33] ibmvfc: check for NULL evt in implicit LOGO and target delete path ---
commit c8380c3ff6514fe4df58a3ad3f0363eddb03454e
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

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

Adds a check for a NULL event in the implicit LOGO and target delete path to
prevent a null pointer dereference in ibmvfc_send_event. If the event is
NULL, it escalates to resetting the host.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 72317d7840a53..59102dd1c7878 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -4406,6 +4406,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: High]
Will this state transition succeed? Looking at ibmvfc_set_tgt_action(),
it appears that transitioning from IBMVFC_TGT_ACTION_LOGOUT_RPORT to
IBMVFC_TGT_ACTION_NONE is silently rejected by the state machine:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_set_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;
	...
}

Since this silently fails and leaves the target in
IBMVFC_TGT_ACTION_LOGOUT_RPORT, does this cause ibmvfc_dev_logo_to_do()
to continually return 1?

If so, the discovery loop could get stuck infinitely retrying the logout
and triggering continuous host resets. Should this error path transition
to IBMVFC_TGT_ACTION_DEL_RPORT instead?

> +		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--;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723000149.969416-1-tyreld@linux.ibm.com?part=3