[ovs-dev] [PATCH net-next v4 08/10] net: openvswitch: fix possible memleak on destroy flow-table

Pravin Shelar pshelar at ovn.org
Thu Oct 17 22:38:35 UTC 2019


On Wed, Oct 16, 2019 at 5:50 AM <xiangxia.m.yue at gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue at gmail.com>
>
> When we destroy the flow tables which may contain the flow_mask,
> so release the flow mask struct.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> Tested-by: Greg Rose <gvrose8192 at gmail.com>
> ---
>  net/openvswitch/flow_table.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 5df5182..d5d768e 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -295,6 +295,18 @@ static void table_instance_destroy(struct table_instance *ti,
>         }
>  }
>
> +static void tbl_mask_array_destroy(struct flow_table *tbl)
> +{
> +       struct mask_array *ma = ovsl_dereference(tbl->mask_array);
> +       int i;
> +
> +       /* Free the flow-mask and kfree_rcu the NULL is allowed. */
> +       for (i = 0; i < ma->max; i++)
> +               kfree_rcu(rcu_dereference_raw(ma->masks[i]), rcu);
> +
> +       kfree_rcu(rcu_dereference_raw(tbl->mask_array), rcu);
> +}
> +
>  /* No need for locking this function is called from RCU callback or
>   * error path.
>   */
> @@ -304,7 +316,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table)
>         struct table_instance *ufid_ti = rcu_dereference_raw(table->ufid_ti);
>
>         free_percpu(table->mask_cache);
> -       kfree_rcu(rcu_dereference_raw(table->mask_array), rcu);
> +       tbl_mask_array_destroy(table);
>         table_instance_destroy(ti, ufid_ti, false);
>  }

This should not be required. mask is linked to a flow and gets
released when flow is removed.
Does the memory leak occur when OVS module is abruptly unloaded and
userspace does not cleanup flow table?
In that case better fix could be calling ovs_flow_tbl_remove()
equivalent from table_instance_destroy when it is iterating flow
table.


More information about the dev mailing list