Re: [PATCH 10/13] scsi: qla2xxx: enable FPIN notification for NVMe

On Wed, Aug 12, 2026 at 2:35 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Lockless access to `fcport->nvme_remote_port` in `qla2x00_set_rport_marginal()` introduces TOCTOU Use-After-Free and NULL pointer dereferences.
> --
>
> --- Patch [10]: [PATCH 10/13] scsi: qla2xxx: enable FPIN notification for NVMe ---
> commit 1b0bf8a2abd4965d07b3b9fd652b0926356c721b
> Author: Jesse Taube <[email protected]>
>
> scsi: qla2xxx: enable FPIN notification for NVMe
>
> Implement set_rport_marginal callback using nvme_fc_set_remoteport_fpin()
> to enable FPIN notifications for NVMe.
>
> > diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
> > index a4ca22024edeb..92db7d584eeb8 100644
> > --- a/drivers/scsi/qla2xxx/qla_attr.c
> > +++ b/drivers/scsi/qla2xxx/qla_attr.c
> > @@ -2789,6 +2789,15 @@ qla2x00_set_rport_loss_tmo(struct fc_rport *rport, uint32_t timeout)
> >                                              rport->dev_loss_tmo);
> >  }
> >
> > +static inline void
> > +qla2x00_set_rport_marginal(struct fc_rport *rport, bool marginal)
> > +{
> > +     fc_port_t *fcport = *(fc_port_t **)rport->dd_data;
> > +
> > +     if (IS_ENABLED(CONFIG_NVME_FC) && fcport && fcport->nvme_remote_port)
> > +             nvme_fc_set_remoteport_fpin(fcport->nvme_remote_port, marginal);
>
> [Severity: High]
> Does this code introduce a Time-Of-Check to Time-Of-Use (TOCTOU) race condition?
>
> The nvme_remote_port pointer is read and dereferenced locklessly here. If a
> device loss event triggers qla_nvme_remoteport_delete() concurrently, it sets
> fcport->nvme_remote_port = NULL and allows the NVMe FC transport to free the
> remote port memory.
>
> If qla2x00_set_rport_marginal() checks that nvme_remote_port is non-NULL but
> is preempted before calling nvme_fc_set_remoteport_fpin(), it will pass a stale
> pointer to freed memory, causing a use-after-free.
>
> Alternatively, because the pointer is fetched without READ_ONCE(), the compiler

This is not a valid solution, there would need to be a lock in
fc_port_t or else we would
get a use-after-free. Unless there is a better solution im unaware of
im going to ignore this
as it already exists in the driver.

Thanks,
Jesse Taube

> could reload the now-NULL pointer from memory, leading to a NULL pointer
> dereference inside nvme_fc_set_remoteport_fpin().
>
> This pre-existing vulnerability is also present in qla2x00_set_rport_loss_tmo()
> just above this new function.
>
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
>