Aaron, Ilya, thank you both for the review. > There is an existing check for recursion in ovs_execute_actions() and in > there we have a log before dropping: > ... > We should have a similar log here so we're not silently dropping. But, > since we now have two places where this check occurs (and it's > complicated because of how deferred actions is processed), maybe it's > better to have a function that does the management:
Good point. v2 extracts the enter/exit logic into ovs_exec_level_enter() and ovs_exec_level_exit() helpers, exactly as you suggested, and uses them in both clone_execute() and ovs_execute_actions(). The net_crit_ratelimited() log is now emitted on every recursion-limit hit regardless of entry point. > There is a different issue here though. The exec_level is used as a > counter for the flow keys during the clone. And if we increment it > unconditionally while not allocating new keys, we'll ran out of key > slots much faster without actually using them. Agreed this is a real concern. As Aaron noted, most deployments nest only once or twice so the practical pressure from this change should be manageable, but decoupling the recursion counter from the key slot index is the right long-term fix. Happy to leave that as a follow-up so this security fix stays focused. v2 posted as a separate thread. Regards, yang zhuorao _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
