> -----Original Message----- > From: dev [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of > wangyunjian via dev > Sent: Thursday, February 17, 2022 11:27 AM > To: Gaëtan Rivet <gr...@u256.net>; <d...@openvswitch.org> > <d...@openvswitch.org>; Ilya Maximets <i.maxim...@ovn.org> > Cc: dingxiaoxiong <dingxiaoxi...@huawei.com> > Subject: Re: [ovs-dev] [PATCH] ofproto: fix use-after-free for "ofproto". > > > > > -----Original Message----- > > From: Gaëtan Rivet [mailto:gr...@u256.net] > > Sent: Thursday, February 17, 2022 1:29 AM > > To: wangyunjian <wangyunj...@huawei.com>; <d...@openvswitch.org> > > <d...@openvswitch.org>; Ilya Maximets <i.maxim...@ovn.org> > > Cc: amore...@redhat.com; dingxiaoxiong <dingxiaoxi...@huawei.com>; 贺 > 鹏 > > <xnhp0...@gmail.com> > > Subject: Re: [ovs-dev] [PATCH] ofproto: fix use-after-free for "ofproto". > > > > On Wed, Feb 16, 2022, at 14:24, wangyunjian wrote: > > >> -----Original Message----- > > >> From: Gaëtan Rivet [mailto:gr...@u256.net] > > >> Sent: Wednesday, February 16, 2022 7:34 PM > > >> To: wangyunjian <wangyunj...@huawei.com>; <d...@openvswitch.org> > > >> <d...@openvswitch.org>; Ilya Maximets <i.maxim...@ovn.org> > > >> Cc: dingxiaoxiong <dingxiaoxi...@huawei.com> > > >> Subject: Re: [ovs-dev] [PATCH] ofproto: fix use-after-free for "ofproto". > > >> > > >> On Fri, Dec 3, 2021, at 12:25, Yunjian Wang via dev wrote: > > >> > When handler threads lookup a "ofproto" and use it, main thread > > >> > maybe remove and free the "ofproto" at the same time. The "ofproto" > > >> > has not been protected well, which can lead to an OVS crash. > > >> > > > >> > This patch fixes this by making the "ofproto" lookup RCU-safe by > > >> > using cmap instead of hmap and moving remove "ofproto" call > > >> > before xlate_txn_commit(). > > >> > > > >> > > >> I don't understand the point of moving the cmap_remove() call > > >> before xlate_txn_commit(). > > > > > > To use of the rcu_synchronize in the xlate_txn_commit to avoid > > > access to the ofproto from other thread through uuid map. > > > > > > > Yes the reason is clear. > > > > But my question is why is it needed? It seems that the ofproto > > lifecycle was written with the assumption that it would still be used while > > being > destroyed. > > > > Can you explain why it needs to be changed? > > I didn't describe the problem clearly before. The main problem is that hmap > variable is not thread safe. The all_ofproto_dpifs_by_uuid variable uses the > hmap structure, which maybe be accessed by main thread and handler threads.
I change the description to "ofproto: fix concurrent when looking up an ofproto by UUID.". And it maybe be clearly understood. How about this? Thanks, Yunjian > > Thanks, > Yunjian > > > > > -- > > Gaetan Rivet > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
