controller: Fix "use after free" issue in statctrl_run().

Issue:
=====
OVN controller main() is caching the "sbrec_mac_binding/sbrec_fdb"
pointers in "mac_cache_mac_binding->mac_bindings/fdbs" which are
used to update timestamp in mac_cache_mb_stats_run()/
mac_cache_fdb_stats_run(). There is a condition where cached
"sbrec_mac_binding/sbrec_fdb" pointers gets freed without
removing the refs from "mac_cache_mac_binding->mac_bindings/fdbs"
which is leading to crash due to "use after free".

This condition can occur due to following reason.
  - en_runtime_data is input to en_mac_cache.
  - en_runtime_data went for full recompute but engine_run()
    is called with recompute_allowed as False due to
    ofctrl_has_backlog() returned True. This caused to avoid
    the recompute in the current engine run and engine_run_aborted
    is set to True. If recompute_allowed is False and
    engine_run_aborted is True then, inc processing engine is
    skipped.
  - If there are any mac_binding/fdb deletes in the same run
    then, entries gets deleted during ovsdb_idl_loop_run() and
    rows are freed in ovsdb_idl_track_clear() but references
    to them are present in mac_cache_data->mac_bindings/fdbs.

Fix:
===
Avoid statctrl_run() when en_mac_cache engine data is invalid.

Fixes: e1ab41e ("controller: Update MAC binding timestamp")
Signed-off-by: Naveen Yerramneni <[email protected]>
Acked-by: Ales Musil <[email protected]>
Signed-off-by: Numan Siddique <[email protected]>

controller/ovn-controller.c

@@ -5596,34 +5596,33 @@ main(int argc, char *argv[])
 
                     stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME,
                                     time_msec());
-                    if (ovnsb_idl_txn) {
-                        if (ofctrl_has_backlog()) {
-                            /* When there are in-flight messages pending to
-                             * ovs-vswitchd, we should hold on recomputing so
-                             * that the previous flow installations won't be
-                             * delayed.  However, we still want to try if
-                             * recompute is not needed and we can quickly
-                             * incrementally process the new changes, to avoid
-                             * unnecessarily forced recomputes later on.  This
-                             * is because the OVSDB change tracker cannot
-                             * preserve tracked changes across iterations.  If
-                             * change tracking is improved, we can simply skip
-                             * this round of engine_run and continue processing
-                             * acculated changes incrementally later when
-                             * ofctrl_has_backlog() returns false. */
-                            engine_run(false);
-                        } else {
-                            engine_run(true);
-                        }
-                    } else {
-                        /* Even if there's no SB DB transaction available,
-                         * try to run the engine so that we can handle any
-                         * incremental changes that don't require a recompute.
-                         * If a recompute is required, the engine will cancel,
-                         * triggerring a full run in the next iteration.
-                         */
-                        engine_run(false);
-                    }
+
+                    /* Recompute is not allowed in following cases: */
+                    /* 1. No ovnsb_idl_txn  */
+                    /* Even if there's no SB DB transaction available,
+                    * try to run the engine so that we can handle any
+                    * incremental changes that don't require a recompute.
+                    * If a recompute is required, the engine will cancel,
+                    * triggerring a full run in the next iteration.
+                    */
+                    /* 2. ofctrl_has_backlog */
+                    /* When there are in-flight messages pending to
+                     * ovs-vswitchd, we should hold on recomputing so
+                     * that the previous flow installations won't be
+                     * delayed.  However, we still want to try if
+                     * recompute is not needed and we can quickly
+                     * incrementally process the new changes, to avoid
+                     * unnecessarily forced recomputes later on.  This
+                     * is because the OVSDB change tracker cannot
+                     * preserve tracked changes across iterations.  If
+                     * change tracking is improved, we can simply skip
+                     * this round of engine_run and continue processing
+                     * acculated changes incrementally later when
+                     * ofctrl_has_backlog() returns false. */
+
+                    bool recompute_allowed = (ovnsb_idl_txn &&
+                                              !ofctrl_has_backlog());
+                    engine_run(recompute_allowed);
                     stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME,
                                    time_msec());
                     if (engine_has_updated()) {
@@ -5753,6 +5752,7 @@ main(int argc, char *argv[])
                         }
                     }
 
+                    mac_cache_data = engine_get_data(&en_mac_cache);
                     if (mac_cache_data) {
                         statctrl_run(ovnsb_idl_txn, mac_cache_data);
                     }