> -----Original Message----- > From: Gaëtan Rivet [mailto:gr...@u256.net] > Sent: Thursday, February 17, 2022 9:31 PM > To: wangyunjian <wangyunj...@huawei.com>; <d...@openvswitch.org> > <d...@openvswitch.org>; Ilya Maximets <i.maxim...@ovn.org>; 贺鹏 > <xnhp0...@gmail.com>; amore...@redhat.com > Cc: dingxiaoxiong <dingxiaoxi...@huawei.com> > Subject: Re: [ovs-dev] [PATCH] ofproto: fix use-after-free for "ofproto". > > On Thu, Feb 17, 2022, at 08:29, wangyunjian wrote: > >> -----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'm ok on the part switching to using CMAP to allow concurrent reads. > That I see the reason and it is fine. > > The part that I don't understand is moving the cmap_remove() call before the > RCU sync. > > As far as I know, the CMAP type does not require that to safely operate. > The writer thread is allowed to call cmap_remove() while a reader is > iterating on > the CMAP to find a node. The only precaution needed is that actual destruction > of the node (freeing) is deferred, which it is. > > So I don't see the reason to move cmap_remove() before the RCU > synchronization, instead of keeping it as it is now. Could you please explain > your > reasoning?
I consider it is more reasonable for the upcall thread cannot find the ofproto when the ofproto will be deleted, no other special consideration. Thanks, Yunjian > > -- > Gaetan Rivet _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
