Friendly ping.
> -----Original Message-----
> From: wangyunjian
> Sent: Friday, December 3, 2021 7:25 PM
> To: d...@openvswitch.org; i.maxim...@ovn.org
> Cc: dingxiaoxiong <dingxiaoxi...@huawei.com>; xudingke
> <xudin...@huawei.com>; wangyunjian <wangyunj...@huawei.com>; Justin
> Pettit <jpet...@ovn.org>
> Subject: [ovs-dev] [PATCH] ofproto: fix use-after-free for "ofproto".
>
> 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().
>
> (gdb) bt
> hmap_next (hmap.h:398)
> seq_wake_waiters (seq.c:326)
> seq_change_protected (seq.c:134)
> seq_change (seq.c:144)
> ofproto_dpif_send_async_msg (ofproto_dpif.c:263)
> process_upcall (ofproto_dpif_upcall.c:1782)
> recv_upcalls (ofproto_dpif_upcall.c:1026)
> udpif_upcall_handler (ofproto/ofproto_dpif_upcall.c:945)
> ovsthread_wrapper (ovs_thread.c:734)
>
> Cc: Justin Pettit <jpet...@ovn.org>
> Fixes: fcb9579be3c7 ("ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user
> action cookie.")
> Signed-off-by: Yunjian Wang <wangyunj...@huawei.com>
> ---
> ofproto/ofproto-dpif.c | 12 ++++++------
> ofproto/ofproto-dpif.h | 2 +-
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index cba49a99e..aa8d32f81 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -216,8 +216,7 @@ static struct hmap all_ofproto_dpifs_by_name =
>
> HMAP_INITIALIZER(&all_ofproto_dpifs_by_name);
>
> /* All existing ofproto_dpif instances, indexed by ->uuid. */
> -static struct hmap all_ofproto_dpifs_by_uuid =
> -
> HMAP_INITIALIZER(&all_ofproto_dpifs_by_uuid);
> +static struct cmap all_ofproto_dpifs_by_uuid = CMAP_INITIALIZER;
>
> static bool ofproto_use_tnl_push_pop = true;
> static void ofproto_unixctl_init(void);
> @@ -1682,7 +1681,7 @@ construct(struct ofproto *ofproto_)
> hmap_insert(&all_ofproto_dpifs_by_name,
> &ofproto->all_ofproto_dpifs_by_name_node,
> hash_string(ofproto->up.name, 0));
> - hmap_insert(&all_ofproto_dpifs_by_uuid,
> + cmap_insert(&all_ofproto_dpifs_by_uuid,
> &ofproto->all_ofproto_dpifs_by_uuid_node,
> uuid_hash(&ofproto->uuid));
> memset(&ofproto->stats, 0, sizeof ofproto->stats);
> @@ -1778,12 +1777,13 @@ destruct(struct ofproto *ofproto_, bool del)
> ofproto->backer->need_revalidate = REV_RECONFIGURE;
> xlate_txn_start();
> xlate_remove_ofproto(ofproto);
> + cmap_remove(&all_ofproto_dpifs_by_uuid,
> + &ofproto->all_ofproto_dpifs_by_uuid_node,
> + uuid_hash(&ofproto->uuid));
> xlate_txn_commit();
>
> hmap_remove(&all_ofproto_dpifs_by_name,
> &ofproto->all_ofproto_dpifs_by_name_node);
> - hmap_remove(&all_ofproto_dpifs_by_uuid,
> - &ofproto->all_ofproto_dpifs_by_uuid_node);
>
> OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
> CLS_FOR_EACH (rule, up.cr, &table->cls) {
> @@ -5781,7 +5781,7 @@ ofproto_dpif_lookup_by_uuid(const struct uuid
> *uuid)
> {
> struct ofproto_dpif *ofproto;
>
> - HMAP_FOR_EACH_WITH_HASH (ofproto,
> all_ofproto_dpifs_by_uuid_node,
> + CMAP_FOR_EACH_WITH_HASH (ofproto,
> all_ofproto_dpifs_by_uuid_node,
> uuid_hash(uuid),
> &all_ofproto_dpifs_by_uuid) {
> if (uuid_equals(&ofproto->uuid, uuid)) {
> return ofproto;
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index 191cfcb0d..fba03f2cc 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -295,7 +295,7 @@ struct ofproto_dpif {
> struct hmap_node all_ofproto_dpifs_by_name_node;
>
> /* In 'all_ofproto_dpifs_by_uuid'. */
> - struct hmap_node all_ofproto_dpifs_by_uuid_node;
> + struct cmap_node all_ofproto_dpifs_by_uuid_node;
>
> struct ofproto up;
> struct dpif_backer *backer;
> --
> 2.27.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
