From 63a4b4d0f054b2a59fb82b8d825ca3e7dd5f28e6 Mon Sep 17 00:00:00 2001 From: Paolo Valerio Date: Mon, 30 Sep 2024 22:50:34 +0200 Subject: [PATCH] dpctl: Do not allow out of range values in ct-set-limits. The ovs_scan() doesn't enforce in-range values and so lsbits are stored in case of out-of-range or negative values. This way negative or values greater than MAX_UINT32 for "default" are all accepted in dpctl_ct_set_limits(), but they will eventually be casted to uint32_t, whereas for zones all the values above are considered invalid. Align their behaviors and extend the tests for checking values out of the range. Signed-off-by: Paolo Valerio Signed-off-by: Aaron Conole --- lib/dpctl.c | 5 +++-- tests/system-traffic.at | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/lib/dpctl.c b/lib/dpctl.c index 77bf4bf537a..2a700f24ab4 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -2169,8 +2169,8 @@ dpctl_ct_set_limits(int argc, const char *argv[], struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits); int i = dp_arg_exists(argc, argv) ? 2 : 1; struct ds ds = DS_EMPTY_INITIALIZER; + unsigned long long default_limit; struct dpif *dpif = NULL; - uint32_t default_limit; int error; if (i >= argc) { @@ -2186,7 +2186,8 @@ dpctl_ct_set_limits(int argc, const char *argv[], /* Parse default limit */ if (!strncmp(argv[i], "default=", 8)) { - if (ovs_scan(argv[i], "default=%"SCNu32, &default_limit)) { + if (str_to_ullong(argv[i] + 8, 10, &default_limit) && + default_limit <= UINT32_MAX) { ct_dpif_push_zone_limit(&zone_limits, OVS_ZONE_LIMIT_DEFAULT_ZONE, default_limit, 0); i++; diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 98754f70739..a04d9611053 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5692,12 +5692,54 @@ priority=100,in_port=2,udp,action=ct(zone=3,commit),1 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) +dnl Test values out of range for the default limit. +dnl Try to set a negative value. +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=-1], [2], [ignore], [dnl +ovs-vswitchd: invalid default limit (Invalid argument) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +dnl Try to set UINT32_MAX. +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=4294967296], [2], [ignore], [dnl +ovs-vswitchd: invalid default limit (Invalid argument) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +dnl Same range checks for zones. +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=1,limit=-1], [2], [ignore], [dnl +ovs-vswitchd: failed to parse field limit (Invalid argument) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=1,limit=4294967296], [2], [ignore], [dnl +ovs-vswitchd: failed to parse field limit (Invalid argument) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +dnl Double check no limits have been applied. +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [],[dnl +default limit=0 +]) + m4_define([UDP_PKT], [m4_join([,], [eth_src=50:54:00:00:00:0$1,eth_dst=50:54:00:00:00:0$2,dl_type=0x0800], [nw_src=10.1.1.$1,nw_dst=10.1.1.$2], [nw_proto=17,nw_ttl=64,nw_frag=no], [udp_src=1,udp_dst=$3])]) +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=1,limit=0]) +pkt=$(ovs-ofctl compose-packet --bare "UDP_PKT([1], [2], [2])") +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=${pkt} actions=resubmit(,0)"]) + +dnl Double check the zl entry exists but no connection was added. +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [],[dnl +default limit=0 +zone=1,limit=0,count=0 +]) + +dnl Remove limit for zone=1. +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1]) + AT_CHECK([ovs-appctl dpctl/ct-set-limits default=3]) AT_CHECK([ovs-appctl dpctl/ct-get-limits], [],[dnl default limit=3