lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 29 Oct 2019 00:37:56 -0700 From: Pravin Shelar <pshelar@....org> To: Tonghao Zhang <xiangxia.m.yue@...il.com> Cc: Greg Rose <gvrose8192@...il.com>, Linux Kernel Network Developers <netdev@...r.kernel.org>, ovs dev <dev@...nvswitch.org> Subject: Re: [PATCH net-next v4 08/10] net: openvswitch: fix possible memleak on destroy flow-table On Sun, Oct 27, 2019 at 11:49 PM Tonghao Zhang <xiangxia.m.yue@...il.com> wrote: > > On Thu, Oct 24, 2019 at 3:14 PM Pravin Shelar <pshelar@....org> wrote: > > > > On Tue, Oct 22, 2019 at 7:35 PM Tonghao Zhang <xiangxia.m.yue@...il.com> wrote: > > > > > > On Tue, Oct 22, 2019 at 2:58 PM Pravin Shelar <pshelar@....org> wrote: > > > > > > ... > > ... > > > struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, > > > @@ -400,10 +458,9 @@ static struct table_instance > > > *table_instance_rehash(struct table_instance *ti, > > > return new_ti; > > > } > > > > > > -int ovs_flow_tbl_flush(struct flow_table *flow_table) > > > +int ovs_flow_tbl_flush(struct flow_table *table) > > > { > > > - struct table_instance *old_ti, *new_ti; > > > - struct table_instance *old_ufid_ti, *new_ufid_ti; > > > + struct table_instance *new_ti, *new_ufid_ti; > > > > > > new_ti = table_instance_alloc(TBL_MIN_BUCKETS); > > > if (!new_ti) > > > @@ -412,16 +469,12 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) > > > if (!new_ufid_ti) > > > goto err_free_ti; > > > > > > - old_ti = ovsl_dereference(flow_table->ti); > > > - old_ufid_ti = ovsl_dereference(flow_table->ufid_ti); > > > + table_instance_destroy(table, true); > > > > > This would destroy running table causing unnecessary flow miss. Lets > > keep current scheme of setting up new table before destroying current > > one. > > > > > - rcu_assign_pointer(flow_table->ti, new_ti); .... ... > /* Must be called with OVS mutex held. */ > void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) > { > @@ -752,17 +794,7 @@ void ovs_flow_tbl_remove(struct flow_table > *table, struct sw_flow *flow) > struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti); > > BUG_ON(table->count == 0); > - hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); > - table->count--; > - if (ovs_identifier_is_ufid(&flow->id)) { > - hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); > - table->ufid_count--; > - } > - > - /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be > - * accessible as long as the RCU read lock is held. > - */ > - flow_mask_remove(table, flow->mask); > + table_instance_remove(table, ti, ufid_ti, flow, true); > } Lets rename table_instance_remove() to imply it is freeing a flow. Otherwise looks good.
Powered by blists - more mailing lists