[PATCH] net: mscc: ocelot: Fix mirror reference leak on VCAP aux resource error

[PATCH] net: mscc: ocelot: Fix mirror reference leak on VCAP aux resource error

[Wentao Liang]

When ocelot_vcap_policer_add() fails, the aux resource setup returns
without releasing the mirror reference taken earlier by
ocelot_mirror_get(). The caller drops the filter without calling
ocelot_vcap_filter_del_aux_resources(), so the reference is leaked.

Release the mirror reference before returning the error.

Fixes: c3d427eac90f ("net: mscc: ocelot: establish functions for handling VCAP aux resources")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/ethernet/mscc/ocelot_vcap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 25e533e91d71..c479af2a423f 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -970,8 +970,11 @@ ocelot_vcap_filter_add_aux_resources(struct ocelot *ocelot,
 	if (filter->block_id == VCAP_IS2 && filter->action.police_ena) {
 		ret = ocelot_vcap_policer_add(ocelot, filter->action.pol_ix,
 					      &filter->action.pol);
-		if (ret)
+		if (ret) {
+			if (filter->action.mirror_ena)
+				ocelot_mirror_put(ocelot);
 			return ret;
+		}
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH] net: mscc: ocelot: Fix mirror reference leak on VCAP aux resource error

[Simon Horman]

On Thu, Sep 17, 2026 at 11:34:44AM +0000, Wentao Liang wrote:
> When ocelot_vcap_policer_add() fails, the aux resource setup returns
> without releasing the mirror reference taken earlier by
> ocelot_mirror_get(). The caller drops the filter without calling
> ocelot_vcap_filter_del_aux_resources(), so the reference is leaked.
> 
> Release the mirror reference before returning the error.
> 
> Fixes: c3d427eac90f ("net: mscc: ocelot: establish functions for handling VCAP aux resources")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/net/ethernet/mscc/ocelot_vcap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
> index 25e533e91d71..c479af2a423f 100644
> --- a/drivers/net/ethernet/mscc/ocelot_vcap.c
> +++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
> @@ -970,8 +970,11 @@ ocelot_vcap_filter_add_aux_resources(struct ocelot *ocelot,
>  	if (filter->block_id == VCAP_IS2 && filter->action.police_ena) {
>  		ret = ocelot_vcap_policer_add(ocelot, filter->action.pol_ix,
>  					      &filter->action.pol);
> -		if (ret)
> +		if (ret) {
> +			if (filter->action.mirror_ena)
> +				ocelot_mirror_put(ocelot);
>  			return ret;
> +		}
>  	}
>  
>  	return 0;

I think this change would be somewhat more robust against future changes to
this function if it was implemented using a goto-driven unwind ladder,
which is the generally preferred in Networking code.

Something like this (compile tested only!):

diff --git a/drivers/net/ethernet/mscc/ocelot_vcap.c b/drivers/net/ethernet/mscc/ocelot_vcap.c
index 25e533e91d71..ce4a3da38c58 100644
--- a/drivers/net/ethernet/mscc/ocelot_vcap.c
+++ b/drivers/net/ethernet/mscc/ocelot_vcap.c
@@ -957,7 +957,7 @@ ocelot_vcap_filter_add_aux_resources(struct ocelot *ocelot,
 				     struct ocelot_vcap_filter *filter,
 				     struct netlink_ext_ack *extack)
 {
-	struct ocelot_mirror *m;
+	struct ocelot_mirror *m = NULL;
 	int ret;
 
 	if (filter->block_id == VCAP_IS2 && filter->action.mirror_ena) {
@@ -971,10 +971,15 @@ ocelot_vcap_filter_add_aux_resources(struct ocelot *ocelot,
 		ret = ocelot_vcap_policer_add(ocelot, filter->action.pol_ix,
 					      &filter->action.pol);
 		if (ret)
-			return ret;
+			goto err_mirror_put;
 	}
 
 	return 0;
+
+err_mirror_put:
+	if (m)
+		ocelot_mirror_put(ocelot);
+	return ret;
 }
 
 static void

-- 
pw-bot: changes-requested

Re: [PATCH] net: mscc: ocelot: Fix mirror reference leak on VCAP aux resource error

[Wentao Liang]

Hi Simon,

Sorry for the very late reply.

You are right that a goto-driven unwind ladder is more robust here: it
keeps the release of resources taken earlier in the function next to
the failure paths, so that a failure added later cannot silently skip
it. I have reworked the patch along those lines, and a v2 follows in a
separate mail. The v2 also records how the issue was found and how it
was tested.

I am going through all the remaining review comments one by one now,
and I will not send patches in bulk again, so that I do not get ahead
of my ability to follow up.

Thanks,
Wentao