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 <[email protected]> Signed-off-by: Aaron Conole <[email protected]>
openvswitch · Oct 9, 2024 · 63a4b4d · 63a4b4d
1 parent b57c1da
commit 63a4b4d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 2 deletions.
diff --git 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
@@ -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