[PATCH net] net: openvswitch: fix potential UAF on meter attach failure

[PATCH net] net: openvswitch: fix potential UAF on meter attach failure

[Ilya Maximets]

While attaching a newly created meter attach_meter() function makes
the new meter visible to other CPUs but can still fail afterwards.
On failure, it detaches the meter back and returns an error.

However, this is an unexpected behavior for the ovs_meter_cmd_set()
that uses a plain kfree(meter) on attach failure without waiting for
RCU readers to stop using it, assuming it was never visible.

This is never a problem for ovs-vswitchd as it always creates meters
before creating any flows that use them.  But the UAF can be triggered
with a custom application using uAPI:

 BUG: KASAN: slab-use-after-free in ovs_meter_execute (net/openvswitch/meter.c:653)
 Read of size 8 at addr ffff88810d152650 by task meter/2508

 Call Trace:
  ovs_meter_execute (net/openvswitch/meter.c:653)
  do_execute_actions (net/openvswitch/actions.c:1407)
  ovs_execute_actions (net/openvswitch/actions.c:1584)
  ovs_packet_cmd_execute (net/openvswitch/datapath.c:703)
  ...
  netlink_sendmsg (af_netlink.c:1900)

 Allocated by task 2519:
  __kasan_kmalloc (mm/kasan/common.c:398 mm/kasan/common.c:415)
  ovs_meter_cmd_set (net/openvswitch/meter.c:422)
  ...
  netlink_sendmsg (af_netlink.c:1900)

 Freed by task 2519:
  kfree (mm/slub.c:2705 mm/slub.c:6405 mm/slub.c:6720)
  ovs_meter_cmd_set (net/openvswitch/meter.c:479)
  ...
  netlink_sendmsg (af_netlink.c:1900)

Fix that by making sure attach_meter() doesn't make the meter visible
until all the checks are done and the function can't fail anymore.

This also makes sure the "hash" value is calculated after the potential
re-sizing of the table.

Reported by Trend Micro's Zero Day Initiative as ZDI-CAN-31642.

Fixes: c7c4c44c9a95 ("net: openvswitch: expand the meters supported number")
Cc: stable@vger.kernel.org
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 net/openvswitch/meter.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
index a02c47277337..4aaeeae3af5b 100644
--- a/net/openvswitch/meter.c
+++ b/net/openvswitch/meter.c
@@ -133,18 +133,10 @@ static void dp_meter_instance_remove(struct dp_meter_instance *ti,
 
 static int attach_meter(struct dp_meter_table *tbl, struct dp_meter *meter)
 {
-	struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
-	u32 hash = meter_hash(ti, meter->id);
+	struct dp_meter_instance *ti;
+	u32 hash;
 	int err;
 
-	/* In generally, slots selected should be empty, because
-	 * OvS uses id-pool to fetch a available id.
-	 */
-	if (unlikely(rcu_dereference_ovsl(ti->dp_meters[hash])))
-		return -EBUSY;
-
-	dp_meter_instance_insert(ti, meter);
-
 	/* That function is thread-safe. */
 	tbl->count++;
 	if (tbl->count >= tbl->max_meters_allowed) {
@@ -152,16 +144,29 @@ static int attach_meter(struct dp_meter_table *tbl, struct dp_meter *meter)
 		goto attach_err;
 	}
 
-	if (tbl->count >= ti->n_meters &&
-	    dp_meter_instance_realloc(tbl, ti->n_meters * 2)) {
-		err = -ENOMEM;
+	ti = rcu_dereference_ovsl(tbl->ti);
+	if (tbl->count >= ti->n_meters) {
+		err = dp_meter_instance_realloc(tbl, ti->n_meters * 2);
+		if (err)
+			goto attach_err;
+
+		ti = rcu_dereference_ovsl(tbl->ti);
+	}
+
+	hash = meter_hash(ti, meter->id);
+
+	/* In general, selected slots should be empty, because
+	 * OvS uses id-pool to fetch available ids.
+	 */
+	if (unlikely(rcu_dereference_ovsl(ti->dp_meters[hash]))) {
+		err = -EBUSY;
 		goto attach_err;
 	}
 
+	dp_meter_instance_insert(ti, meter);
 	return 0;
 
 attach_err:
-	dp_meter_instance_remove(ti, meter);
 	tbl->count--;
 	return err;
 }
-- 
2.55.0

Re: [PATCH net] net: openvswitch: fix potential UAF on meter attach failure

[Eelco Chaudron]

On 27 Jul 2026, at 14:10, Ilya Maximets wrote:

> While attaching a newly created meter attach_meter() function makes
> the new meter visible to other CPUs but can still fail afterwards.
> On failure, it detaches the meter back and returns an error.
>
> However, this is an unexpected behavior for the ovs_meter_cmd_set()
> that uses a plain kfree(meter) on attach failure without waiting for
> RCU readers to stop using it, assuming it was never visible.
>
> This is never a problem for ovs-vswitchd as it always creates meters
> before creating any flows that use them.  But the UAF can be triggered
> with a custom application using uAPI:
>
>  BUG: KASAN: slab-use-after-free in ovs_meter_execute (net/openvswitch/meter.c:653)
>  Read of size 8 at addr ffff88810d152650 by task meter/2508
>
>  Call Trace:
>   ovs_meter_execute (net/openvswitch/meter.c:653)
>   do_execute_actions (net/openvswitch/actions.c:1407)
>   ovs_execute_actions (net/openvswitch/actions.c:1584)
>   ovs_packet_cmd_execute (net/openvswitch/datapath.c:703)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
>
>  Allocated by task 2519:
>   __kasan_kmalloc (mm/kasan/common.c:398 mm/kasan/common.c:415)
>   ovs_meter_cmd_set (net/openvswitch/meter.c:422)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
>
>  Freed by task 2519:
>   kfree (mm/slub.c:2705 mm/slub.c:6405 mm/slub.c:6720)
>   ovs_meter_cmd_set (net/openvswitch/meter.c:479)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
>
> Fix that by making sure attach_meter() doesn't make the meter visible
> until all the checks are done and the function can't fail anymore.
>
> This also makes sure the "hash" value is calculated after the potential
> re-sizing of the table.
>
> Reported by Trend Micro's Zero Day Initiative as ZDI-CAN-31642.
>
> Fixes: c7c4c44c9a95 ("net: openvswitch: expand the meters supported number")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Thanks, Ilya, for looking into this. The patch looks good to me.

Reviewed-by:  Eelco Chaudron <echaudro@redhat.com>

Re: [PATCH net] net: openvswitch: fix potential UAF on meter attach failure

[Ilya Maximets]

On 7/27/26 2:10 PM, Ilya Maximets wrote:
> While attaching a newly created meter attach_meter() function makes
> the new meter visible to other CPUs but can still fail afterwards.
> On failure, it detaches the meter back and returns an error.
> 
> However, this is an unexpected behavior for the ovs_meter_cmd_set()
> that uses a plain kfree(meter) on attach failure without waiting for
> RCU readers to stop using it, assuming it was never visible.
> 
> This is never a problem for ovs-vswitchd as it always creates meters
> before creating any flows that use them.  But the UAF can be triggered
> with a custom application using uAPI:
> 
>  BUG: KASAN: slab-use-after-free in ovs_meter_execute (net/openvswitch/meter.c:653)
>  Read of size 8 at addr ffff88810d152650 by task meter/2508
> 
>  Call Trace:
>   ovs_meter_execute (net/openvswitch/meter.c:653)
>   do_execute_actions (net/openvswitch/actions.c:1407)
>   ovs_execute_actions (net/openvswitch/actions.c:1584)
>   ovs_packet_cmd_execute (net/openvswitch/datapath.c:703)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
> 
>  Allocated by task 2519:
>   __kasan_kmalloc (mm/kasan/common.c:398 mm/kasan/common.c:415)
>   ovs_meter_cmd_set (net/openvswitch/meter.c:422)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
> 
>  Freed by task 2519:
>   kfree (mm/slub.c:2705 mm/slub.c:6405 mm/slub.c:6720)
>   ovs_meter_cmd_set (net/openvswitch/meter.c:479)
>   ...
>   netlink_sendmsg (af_netlink.c:1900)
> 
> Fix that by making sure attach_meter() doesn't make the meter visible
> until all the checks are done and the function can't fail anymore.
> 
> This also makes sure the "hash" value is calculated after the potential
> re-sizing of the table.
> 
> Reported by Trend Micro's Zero Day Initiative as ZDI-CAN-31642.
> 
> Fixes: c7c4c44c9a95 ("net: openvswitch: expand the meters supported number")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Just addressing some sashiko comments below.  They are either insignificant
or for other pre-existing issues:

>  net/openvswitch/meter.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> index a02c47277337..4aaeeae3af5b 100644
> --- a/net/openvswitch/meter.c
> +++ b/net/openvswitch/meter.c
> @@ -133,18 +133,10 @@ static void dp_meter_instance_remove(struct dp_meter_instance *ti,
>  
>  static int attach_meter(struct dp_meter_table *tbl, struct dp_meter *meter)
>  {
> -	struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
> -	u32 hash = meter_hash(ti, meter->id);
> +	struct dp_meter_instance *ti;
> +	u32 hash;
>  	int err;
>  
> -	/* In generally, slots selected should be empty, because
> -	 * OvS uses id-pool to fetch a available id.
> -	 */
> -	if (unlikely(rcu_dereference_ovsl(ti->dp_meters[hash])))
> -		return -EBUSY;
> -
> -	dp_meter_instance_insert(ti, meter);
> -
>  	/* That function is thread-safe. */
>  	tbl->count++;

Sashiko complains about the comment, but it is fine.  It is pointing out
that it's safe to increment the count, which is still the case.

>  	if (tbl->count >= tbl->max_meters_allowed) {
> @@ -152,16 +144,29 @@ static int attach_meter(struct dp_meter_table *tbl, struct dp_meter *meter)
>  		goto attach_err;
>  	}
>  
> -	if (tbl->count >= ti->n_meters &&
> -	    dp_meter_instance_realloc(tbl, ti->n_meters * 2)) {
> -		err = -ENOMEM;
> +	ti = rcu_dereference_ovsl(tbl->ti);
> +	if (tbl->count >= ti->n_meters) {
> +		err = dp_meter_instance_realloc(tbl, ti->n_meters * 2);

Both sashiko instances point out that realloc doesn't reshash the entries,
which is a known behavior of the meter table.  It expects meters to be added
sequentially and not with the random ids, as that is how ovs-vswitchd
operates.  This is not a good assumption to have and the table should be
replaced by a real hash table, but it is a separate work item.

> +		if (err)
> +			goto attach_err;
> +
> +		ti = rcu_dereference_ovsl(tbl->ti);
> +	}
> +
> +	hash = meter_hash(ti, meter->id);
> +
> +	/* In general, selected slots should be empty, because
> +	 * OvS uses id-pool to fetch available ids.
> +	 */
> +	if (unlikely(rcu_dereference_ovsl(ti->dp_meters[hash]))) {
> +		err = -EBUSY;
>  		goto attach_err;
>  	}
>  
> +	dp_meter_instance_insert(ti, meter);

It also complains that the caller relies on the fact that attach_meter()
doesn't make the pointer visible.  However, IMO, this is the expected
behavior from this function and doesn't require extra comments.  The
opposite behavior would warrant a comment.

>  	return 0;
>  
>  attach_err:
> -	dp_meter_instance_remove(ti, meter);
>  	tbl->count--;
>  	return err;
>  }

Re: [PATCH net] net: openvswitch: fix potential UAF on meter attach failure

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 27 Jul 2026 14:10:21 +0200 you wrote:
> While attaching a newly created meter attach_meter() function makes
> the new meter visible to other CPUs but can still fail afterwards.
> On failure, it detaches the meter back and returns an error.
> 
> However, this is an unexpected behavior for the ovs_meter_cmd_set()
> that uses a plain kfree(meter) on attach failure without waiting for
> RCU readers to stop using it, assuming it was never visible.
> 
> [...]

Here is the summary with links:
  - [net] net: openvswitch: fix potential UAF on meter attach failure
    https://git.kernel.org/netdev/net/c/a58a2b0ce354

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html