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

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

[Wentao Liang]

mpi3mr_get_tgtdev_by_handle() takes a reference on the target device
and the loop drops it with mpi3mr_tgtdev_put() at the end of every
iteration. The MPI3_EVENT_SAS_TOPO_PHY_RC_TARG_NOT_RESPONDING branch
drops the same reference a second time before breaking out, so the
reference is put twice and the kref can be released prematurely.

Drop the redundant put and let the loop release the reference once.

Fixes: 13ef29ea4aa0 ("scsi: mpi3mr: Add support for device add/remove event handling")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/scsi/mpi3mr/mpi3mr_os.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 0c974f2627d1..5ff5720f2491 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -1869,7 +1869,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;
 		case MPI3_EVENT_SAS_TOPO_PHY_RC_RESPONDING:
 		case MPI3_EVENT_SAS_TOPO_PHY_RC_PHY_CHANGED:
-- 
2.34.1

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

[sashiko-bot]

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