summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Loeliger <jdl@netgate.com>2022-09-12 12:41:06 -0500
committerNeale Ranns <neale@graphiant.com>2022-09-19 01:39:05 +0000
commit755b529c11d37f839dfba91127657a47390b88a2 (patch)
tree0fc26749d794d070c721e65ce4768b80021610c1
parent6e3b3b76727c629b478e9ff9ed8836e595650a18 (diff)
abf: add API parameter n_paths range checks
Also check for non-zero rpath length in CLI cmd. While there, no need to use "else" after a return. Also while there, notice and fix numerous input_line buffer leaks and fix them. Type: fix Fixes: 669d07dc016757b856e1014a415996cf9f0ebc58 Signed-off-by: Jon Loeliger <jdl@netgate.com> Change-Id: I18ea44b7b82e8938c3e793e7c2a04dfe157076d8
-rw-r--r--src/plugins/abf/abf.api2
-rw-r--r--src/plugins/abf/abf_api.c6
-rw-r--r--src/plugins/abf/abf_policy.c91
3 files changed, 56 insertions, 43 deletions
diff --git a/src/plugins/abf/abf.api b/src/plugins/abf/abf.api
index 1cd3da7e557..a748de4522b 100644
--- a/src/plugins/abf/abf.api
+++ b/src/plugins/abf/abf.api
@@ -51,7 +51,7 @@ define abf_plugin_get_version_reply
/** \brief A description of an ABF policy
@param policy_id User chosen Identifier for the policy
@param acl_index The ACL that the policy will match against
- @param n_paths Number of paths
+ @param n_paths Number of paths, 1..255
@param paths The set of forwarding paths that are being added or removed.
*/
typedef abf_policy
diff --git a/src/plugins/abf/abf_api.c b/src/plugins/abf/abf_api.c
index 66121acbdf0..222e1f4e116 100644
--- a/src/plugins/abf/abf_api.c
+++ b/src/plugins/abf/abf_api.c
@@ -69,6 +69,12 @@ vl_api_abf_policy_add_del_t_handler (vl_api_abf_policy_add_del_t * mp)
int rv = 0;
u8 pi;
+ if (mp->policy.n_paths == 0)
+ {
+ rv = VNET_API_ERROR_INVALID_VALUE;
+ goto done;
+ }
+
vec_validate (paths, mp->policy.n_paths - 1);
for (pi = 0; pi < mp->policy.n_paths; pi++)
diff --git a/src/plugins/abf/abf_policy.c b/src/plugins/abf/abf_policy.c
index 1921df113ff..8f0fc116144 100644
--- a/src/plugins/abf/abf_policy.c
+++ b/src/plugins/abf/abf_policy.c
@@ -192,50 +192,45 @@ abf_policy_delete (u32 policy_id, const fib_route_path_t * rpaths)
*/
return (VNET_API_ERROR_INVALID_VALUE);
}
- else
- {
- /*
- * update an existing policy.
- * - add the path to the path-list and swap our ancestry
- * - backwalk to poke all attachments to update
- */
- fib_node_index_t old_pl;
- ap = abf_policy_get (api);
- old_pl = ap->ap_pl;
+ /*
+ * update an existing policy.
+ * - add the path to the path-list and swap our ancestry
+ * - backwalk to poke all attachments to update
+ */
+ fib_node_index_t old_pl;
- fib_path_list_lock (old_pl);
- ap->ap_pl =
- fib_path_list_copy_and_path_remove (ap->ap_pl,
- (FIB_PATH_LIST_FLAG_SHARED |
- FIB_PATH_LIST_FLAG_NO_URPF),
- rpaths);
+ ap = abf_policy_get (api);
+ old_pl = ap->ap_pl;
- fib_path_list_child_remove (old_pl, ap->ap_sibling);
- ap->ap_sibling = ~0;
+ fib_path_list_lock (old_pl);
+ ap->ap_pl = fib_path_list_copy_and_path_remove (
+ ap->ap_pl, (FIB_PATH_LIST_FLAG_SHARED | FIB_PATH_LIST_FLAG_NO_URPF),
+ rpaths);
- if (FIB_NODE_INDEX_INVALID == ap->ap_pl)
- {
- /*
- * no more paths on this policy. It's toast
- * remove the CLI/API's lock
- */
- fib_node_unlock (&ap->ap_node);
- }
- else
- {
- ap->ap_sibling = fib_path_list_child_add (ap->ap_pl,
- abf_policy_fib_node_type,
- api);
+ fib_path_list_child_remove (old_pl, ap->ap_sibling);
+ ap->ap_sibling = ~0;
- fib_node_back_walk_ctx_t ctx = {
- .fnbw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE,
- };
+ if (FIB_NODE_INDEX_INVALID == ap->ap_pl)
+ {
+ /*
+ * no more paths on this policy. It's toast
+ * remove the CLI/API's lock
+ */
+ fib_node_unlock (&ap->ap_node);
+ }
+ else
+ {
+ ap->ap_sibling =
+ fib_path_list_child_add (ap->ap_pl, abf_policy_fib_node_type, api);
- fib_walk_sync (abf_policy_fib_node_type, api, &ctx);
- }
- fib_path_list_unlock (old_pl);
+ fib_node_back_walk_ctx_t ctx = {
+ .fnbw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE,
+ };
+
+ fib_walk_sync (abf_policy_fib_node_type, api, &ctx);
}
+ fib_path_list_unlock (old_pl);
return (0);
}
@@ -272,14 +267,25 @@ abf_policy_cmd (vlib_main_t * vm,
unformat_fib_route_path, &rpath, &payload_proto))
vec_add1 (rpaths, rpath);
else
- return (clib_error_return (0, "unknown input '%U'",
- format_unformat_error, line_input));
+ {
+ clib_error_t *err;
+ err = clib_error_return (0, "unknown input '%U'",
+ format_unformat_error, line_input);
+ unformat_free (line_input);
+ return err;
+ }
}
if (INDEX_INVALID == policy_id)
{
vlib_cli_output (vm, "Specify a Policy ID");
- return 0;
+ goto out;
+ }
+
+ if (vec_len (rpaths) == 0)
+ {
+ vlib_cli_output (vm, "Hop path must not be empty");
+ goto out;
}
if (!is_del)
@@ -287,7 +293,7 @@ abf_policy_cmd (vlib_main_t * vm,
if (INDEX_INVALID == acl_index)
{
vlib_cli_output (vm, "ACL index must be set");
- return 0;
+ goto out;
}
rv = abf_policy_update (policy_id, acl_index, rpaths);
@@ -296,7 +302,7 @@ abf_policy_cmd (vlib_main_t * vm,
{
vlib_cli_output (vm,
"ACL index must match existing ACL index in policy");
- return 0;
+ goto out;
}
}
else
@@ -304,6 +310,7 @@ abf_policy_cmd (vlib_main_t * vm,
abf_policy_delete (policy_id, rpaths);
}
+out:
unformat_free (line_input);
return (NULL);
}