Re: [PATCH 09/17] mpi3mr: Fix target device reference leak in device removal handshake

On Tue, Aug 18, 2026 at 11:48 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Memory leak of tgt_dev reference in mpi3mr_target_alloc() error paths.
> - [High] Memory leak of tgtdev reference in mpi3mr_get_port_id_by_rphy() when hba_port is NULL.
> - [High] Memory leak of tgtdev reference in mpi3mr_sas_port_add() error paths.
> --
>
> --- Patch [9]: [PATCH 09/17] mpi3mr: Fix target device reference leak in device removal handshake ---
> commit d4f3aa04629aa5ef9b7db8bd5ef68d81cfceef82
> Author: Chandrakanth Patil <[email protected]>
>
> mpi3mr: Fix target device reference leak in device removal handshake
>
> This commit fixes a target device reference leak in the device removal
> handshake. It drops the reference acquired by the device lookup once the
> state has been updated.
>
> > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> > index 7ae4b4fad3f4a..b18e7ea873030 100644
> > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> > @@ -2522,6 +2522,9 @@ static void mpi3mr_dev_rmhs_send_tm(struct mpi3mr_ioc *mrioc, u16 handle,
> >               tgtdev->state = MPI3MR_DEV_REMOVE_HS_STARTED;
> >       spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
> >
> > +     if (tgtdev)
> > +             mpi3mr_tgtdev_put(tgtdev);
> > +
> >       if (drv_cmd)
> >               goto issue_cmd;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but since this commit fixes a
> target device reference leak, I noticed a similar pre-existing leak in
> mpi3mr_target_alloc() in drivers/scsi/mpi3mr/mpi3mr_os.c.
>
> If the validation checks fail, the function sets retval = -ENXIO and exits
> without releasing the acquired tgt_dev reference:
>
> drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_target_alloc() {
>     ...
>     tgt_dev = __mpi3mr_get_tgtdev_by_perst_id(mrioc, starget->id);
>     if (tgt_dev && !tgt_dev->is_hidden && tgt_dev->non_stl) {
>         ...
>     } else {
>         retval = -ENXIO;
>     }
>     ...
> }
>
> Does this cause a permanent memory leak of the target device structure?
>
>
> [Severity: High]
> This is a pre-existing issue, but there appears to be another leak in
> mpi3mr_get_port_id_by_rphy() in drivers/scsi/mpi3mr/mpi3mr_transport.c.
>
> The code retrieves the target device and takes a reference, but it only
> puts the reference inside the condition block that checks if the device's
> hba_port is non-NULL:
>
> drivers/scsi/mpi3mr/mpi3mr_transport.c:mpi3mr_get_port_id_by_rphy() {
>     ...
>     tgtdev = __mpi3mr_get_tgtdev_by_addr_and_rphy(mrioc, ...);
>     if (tgtdev && tgtdev->dev_spec.sas_sata_inf.hba_port) {
>         ...
>         mpi3mr_tgtdev_put(tgtdev);
>     }
>     ...
> }
>
> If a device is found but its hba_port is NULL, the function exits without
> releasing the reference. Does this result in a persistent memory leak?
>
>
> [Severity: High]
> This is also a pre-existing issue, but does mpi3mr_sas_port_add() in
> drivers/scsi/mpi3mr/mpi3mr_transport.c leak a reference during error paths?
>
> When adding an end device, a reference to the target device is acquired. If
> any subsequent setup or allocation fails, the code jumps to the out_fail
> label. The cleanup path handles freeing the SAS port structure but seems to
> bypass calling mpi3mr_tgtdev_put():
>
> drivers/scsi/mpi3mr/mpi3mr_transport.c:mpi3mr_sas_port_add() {
>     ...
>     if (rphy->identify.device_type == SAS_END_DEVICE) {
>         tgtdev = mpi3mr_get_tgtdev_by_addr(...);
>     }
>     ...
> out_fail:
>     ...
> }
>
> Could this leak the target device reference during port addition failures?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Thanks for the review.

New issues:
- None. This patch specifically fixes a target device reference leak
in the device removal handshake by calling mpi3mr_tgtdev_put().

Pre-existing issues:
- The other reference leaks pointed out (in mpi3mr_target_alloc(),
mpi3mr_get_port_id_by_rphy(), and mpi3mr_sas_port_add()) are
  pre-existing in the driver and will be addressed in a separate
follow-up patch series.