binding.c: update ld->peers when lsp type updated

The local_datapath->peer_ports list contains peers pointers
to lsp<-->lrp ports that are supposed to be router end ports,
those pointers are added and deleted to the  local_datapath->peer_ports
when logical switch port of type router are added or deleted from the database.

The deletion and creation of those ports are handled very well when the LSP type
is a router, but if in any case, the user has changed the LSP type from router
port to any other LSP type the ld->peer_ports will keep pointing to this port
and if it was deleted it will keep pointing to invalid memory regions and that
could lead to invalid memory access in the ovn-controller.

To solve the above issue this patch will update the local_dataoath->peer_ports
whenever a lport is updated.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2077078
Co-authored-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>

controller/binding.c

@@ -549,6 +549,39 @@ remove_related_lport(const struct sbrec_port_binding *pb,
     }
 }
 
+/*
+ * Update local_datapath peers when port type changed
+ * and remove irrelevant ports from this list.
+ */
+static void
+update_ld_peers(const struct sbrec_port_binding *pb,
+                 struct hmap *local_datapaths)
+{
+    struct local_datapath *ld =
+        get_local_datapath(local_datapaths, pb->datapath->tunnel_key);
+
+    if (!ld) {
+        return;
+    }
+
+    /*
+     * This will handle cases where the pb type was explicitly
+     * changed from router type to any other port type and will
+     * remove it from the ld peers list.
+     */
+    enum en_lport_type type = get_lport_type(pb);
+    int num_peers = ld->n_peer_ports;
+    if (type != LP_PATCH) {
+        remove_local_datapath_peer_port(pb, ld, local_datapaths);
+        if (num_peers != ld->n_peer_ports) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_DBG_RL(&rl,
+                        "removing lport %s from the ld peers list",
+                        pb->logical_port);
+        }
+    }
+}
+
 static void
 delete_active_pb_ras_pd(const struct sbrec_port_binding *pb,
                         struct shash *ras_pd_map)
@@ -2398,6 +2431,10 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
         return true;
     }
 
+    if (sbrec_port_binding_is_updated(pb, SBREC_PORT_BINDING_COL_TYPE)) {
+        update_ld_peers(pb, b_ctx_out->local_datapaths);
+    }
+
     update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ipv6_pd,
                             "ipv6_prefix_delegation");
 

tests/ovn.at

@@ -30334,3 +30334,44 @@ check test "$snat_zone" = "$SNAT_ZONE_REG"
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([router port type update and then remove])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+    set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lr-add ro0
+check ovn-nbctl lsp-add sw0 sw0-p1
+check ovn-nbctl lsp-add sw0 lsp
+check ovn-nbctl lsp-set-type lsp router
+check ovn-nbctl lsp-set-options lsp router-port=lrp
+check ovn-nbctl lsp-set-addresses lsp  00:00:00:00:00:1
+check ovn-nbctl lrp-add ro0 lrp 00:00:00:00:00:1 aef0:0:0:0:0:0:0:1/64
+check ovn-nbctl set Logical_Router_Port lrp ipv6_ra_configs:send_periodic=true \
+        -- set Logical_Router_Port lrp ipv6_ra_configs:address_mode=slaac \
+        -- set Logical_Router_Port lrp ipv6_ra_configs:mtu=1280 \
+        -- set Logical_Router_Port lrp ipv6_ra_configs:max_interval=2 \
+        -- set Logical_Router_Port lrp ipv6_ra_configs:min_interval=1
+
+# The below commands change the LSP from "router" type in order to ensure that
+# no incorrect memory accesses occur. The test passes because there is no crash
+# in ovn-controller.
+check ovn-nbctl lsp-set-type lsp localnet
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl lsp-del lsp
+check ovn-nbctl lrp-del lrp
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])