Re: [PATCH] scsi: mpi3mr: Fix tgtdev reference count in mpi3mr_sastopochg_evt_bh()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: mpi3mr: memory leak in mpi3mr_sastopochg_evt_bh()
--

commit cf5778c62c15425d51079879286e5084420ef1e2
Author: Wentao Liang <vulab@iscas.ac.cn>

scsi: mpi3mr: Fix tgtdev reference count in mpi3mr_sastopochg_evt_bh()

This commit removes an allegedly redundant mpi3mr_tgtdev_put() in
mpi3mr_sastopochg_evt_bh() during SAS topology changes. The patch assumes
that the mpi3mr_tgtdev_put() at the end of the loop is sufficient to release
the reference taken by mpi3mr_get_tgtdev_by_handle().

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index 991e29a8d382f..651e5364a7abd 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -1895,7 +1895,6 @@ static void mpi3mr_sastopochg_evt_bh(struct mpi3mr_ioc *mrioc,
>  			if (tgtdev->host_exposed)
>  				mpi3mr_remove_tgtdev_from_host(mrioc, tgtdev);
>  			mpi3mr_tgtdev_del_from_list(mrioc, tgtdev, false);
> -			mpi3mr_tgtdev_put(tgtdev);
>  			break;

[Severity: Medium]
Will removing this mpi3mr_tgtdev_put() introduce a memory leak of the struct
mpi3mr_tgt_dev?

When a new device is added, the baseline refcount appears to be 2, with one
reference from creation in mpi3mr_alloc_tgtdev(), and one from the list in
mpi3mr_tgtdev_add_to_list():

drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_update_device_info() {
    ...
            mpi3mr_update_tgtdev(mrioc, tgtdev, dev_pg0, true);
            mpi3mr_tgtdev_add_to_list(mrioc, tgtdev);
    ...
}

In mpi3mr_sastopochg_evt_bh(), mpi3mr_get_tgtdev_by_handle() takes a
temporary lookup reference, so the refcount becomes 3.

The call to mpi3mr_tgtdev_del_from_list() will drop the list reference, so
the refcount drops back to 2.

Without the explicit mpi3mr_tgtdev_put() that was removed here, the remaining
mpi3mr_tgtdev_put() at the bottom of the loop only drops the lookup reference
taken by mpi3mr_get_tgtdev_by_handle().

Doesn't this leave the initial creation reference permanently held, leading
to a leak of the mpi3mr_tgt_dev structure whenever a SAS target device is
removed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917150436.2158151-1-vulab@iscas.ac.cn?part=1