Re: [PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled error path leaves the state machine stalled, leading to a leaked fcxp structure and a list_del() crash upon port offline.
--

commit 61029f85cbc6b31d41ea7d38cd2303b673cd3f35
Author: Arnd Bergmann <arnd@arndb.de>

scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

bfa_fcs_fdmi_get_portattr() gets inlined into multiple places and has two
fairly large variables on the stack, to the point of causing a warning
in some randconfig builds. Mark the inner function as noinline_for_stack
to keep it separate from the other variables.

> diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
> index 2df399c537c13..8c9d423129c07 100644
> --- a/drivers/scsi/bfa/bfa_fcs_lport.c
> +++ b/drivers/scsi/bfa/bfa_fcs_lport.c
> @@ -2627,7 +2627,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
>  
>  }
>  
> -static void
> +static noinline_for_stack void
>  bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
>  			  struct bfa_fcs_fdmi_port_attr_s *port_attr)
>  {

[Severity: High]
This is a pre-existing issue, but I have a question about the error path in
bfa_fcs_lport_fdmi_send_rhba() when it calls bfa_fcs_lport_fdmi_build_rhba_pyld():

drivers/scsi/bfa/bfa_fcs_lport.c:bfa_fcs_lport_fdmi_send_rhba() {
    ...
	attr_len =
		bfa_fcs_lport_fdmi_build_rhba_pyld(fdmi,
					  (u8 *) ((struct ct_hdr_s *) pyld
						       + 1));
	if (attr_len < 0)
		return;
    ...
}

If this returns early due to a negative error code, does it leak the
successfully allocated fcxp structure?

It appears that returning early here fails to free the fcxp structure and does
not advance the state machine, leaving it stalled in the
bfa_fcs_lport_fdmi_sm_sending_rhba state.

If a port offline event subsequently occurs, the state machine blindly cancels
the allocation:

drivers/scsi/bfa/bfa_fcs_lport.c:bfa_fcs_lport_fdmi_sm_sending_rhba() {
    ...
	case FDMISM_EVENT_PORT_OFFLINE:
		bfa_sm_set_state(fdmi, bfa_fcs_lport_fdmi_sm_offline);
		bfa_fcxp_walloc_cancel(BFA_FCS_GET_HAL_FROM_PORT(port),
					   &fdmi->fcxp_wqe);
		break;
    ...
}

Since fcxp was originally successfully allocated, the wait queue entry
(fdmi->fcxp_wqe.qe) was never linked onto the wait list. Can calling
bfa_fcxp_walloc_cancel() on this unqueued and uninitialized element trigger a
list_del() crash?

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