pipeline: fix string copy into fixed size buffer

Message ID 20201026195916.8325-1-cristian.dumitrescu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series pipeline: fix string copy into fixed size buffer |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Cristian Dumitrescu Oct. 26, 2020, 7:59 p.m. UTC
  Fix potential buffer overflows by string copy into fixed size buffer.

Coverity issue: 362732, 362736, 362760, 362772, 362775, 362784,
                362800, 362803, 362806, 362811, 362814, 362816,
		362834, 362837, 362844, 362845, 362857, 362861,
		362868, 362890, 362893, 362904, 362905.

Fixes: 56492fd536 ("pipeline: add new SWX pipeline type")
Fixes: 1e4c88caea ("pipeline: add SWX extern objects and funcs")
Fixes: e9d870dd93 ("pipeline: add SWX pipeline tables")
Fixes: a1711f948d ("pipeline: add SWX Rx and extract instructions")

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 lib/librte_pipeline/rte_swx_pipeline.c      | 28 +++++++++++++++------
 lib/librte_pipeline/rte_swx_pipeline.h      | 11 ++++++++
 lib/librte_pipeline/rte_swx_pipeline_spec.c | 20 +++++++++++----
 3 files changed, 46 insertions(+), 13 deletions(-)
  

Comments

David Marchand Oct. 29, 2020, 4:31 p.m. UTC | #1
On Mon, Oct 26, 2020 at 8:59 PM Cristian Dumitrescu
<cristian.dumitrescu@intel.com> wrote:
>
> Fix potential buffer overflows by string copy into fixed size buffer.
>
> Coverity issue: 362732, 362736, 362760, 362772, 362775, 362784,
>                 362800, 362803, 362806, 362811, 362814, 362816,
>                 362834, 362837, 362844, 362845, 362857, 362861,
>                 362868, 362890, 362893, 362904, 362905.

This sets a record for the project :-).

> Fixes: 56492fd536 ("pipeline: add new SWX pipeline type")
> Fixes: 1e4c88caea ("pipeline: add SWX extern objects and funcs")
> Fixes: e9d870dd93 ("pipeline: add SWX pipeline tables")
> Fixes: a1711f948d ("pipeline: add SWX Rx and extract instructions")
>
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks.
  

Patch

diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
index 9d6461101..58480788b 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.c
+++ b/lib/librte_pipeline/rte_swx_pipeline.c
@@ -22,7 +22,17 @@  do {                                                                           \
 } while (0)
 
 #define CHECK_NAME(name, err_code)                                             \
-	CHECK((name) && (name)[0], err_code)
+	CHECK((name) &&                                                        \
+	      (name)[0] &&                                                     \
+	      (strnlen((name), RTE_SWX_NAME_SIZE) < RTE_SWX_NAME_SIZE),        \
+	      err_code)
+
+#define CHECK_INSTRUCTION(instr, err_code)                                     \
+	CHECK((instr) &&                                                       \
+	      (instr)[0] &&                                                    \
+	      (strnlen((instr), RTE_SWX_INSTRUCTION_SIZE) <                    \
+	       RTE_SWX_INSTRUCTION_SIZE),                                      \
+	      err_code)
 
 #ifndef TRACE_LEVEL
 #define TRACE_LEVEL 0
@@ -1635,12 +1645,12 @@  rte_swx_pipeline_extern_type_member_func_register(struct rte_swx_pipeline *p,
 
 	CHECK(p, EINVAL);
 
-	CHECK(extern_type_name, EINVAL);
+	CHECK_NAME(extern_type_name, EINVAL);
 	type = extern_type_find(p, extern_type_name);
 	CHECK(type, EINVAL);
 	CHECK(type->n_funcs < RTE_SWX_EXTERN_TYPE_MEMBER_FUNCS_MAX, ENOSPC);
 
-	CHECK(name, EINVAL);
+	CHECK_NAME(name, EINVAL);
 	CHECK(!extern_type_member_func_find(type, name), EEXIST);
 
 	CHECK(member_func, EINVAL);
@@ -5280,8 +5290,6 @@  instr_return_exec(struct rte_swx_pipeline *p)
 	t->ip = t->ret;
 }
 
-#define RTE_SWX_INSTRUCTION_TOKENS_MAX 16
-
 static int
 instr_translate(struct rte_swx_pipeline *p,
 		struct action *action,
@@ -5301,6 +5309,7 @@  instr_translate(struct rte_swx_pipeline *p,
 			break;
 
 		CHECK(n_tokens < RTE_SWX_INSTRUCTION_TOKENS_MAX, EINVAL);
+		CHECK_NAME(token, EINVAL);
 
 		tokens[n_tokens] = token;
 		n_tokens++;
@@ -5938,7 +5947,7 @@  instruction_config(struct rte_swx_pipeline *p,
 	CHECK(n_instructions, EINVAL);
 	CHECK(instructions, EINVAL);
 	for (i = 0; i < n_instructions; i++)
-		CHECK(instructions[i], EINVAL);
+		CHECK_INSTRUCTION(instructions[i], EINVAL);
 
 	/* Memory allocation. */
 	instr = calloc(n_instructions, sizeof(struct instruction));
@@ -6442,7 +6451,7 @@  rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 		struct action *a;
 		uint32_t action_data_size;
 
-		CHECK(action_name, EINVAL);
+		CHECK_NAME(action_name, EINVAL);
 
 		a = action_find(p, action_name);
 		CHECK(a, EINVAL);
@@ -6452,7 +6461,7 @@  rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 			action_data_size_max = action_data_size;
 	}
 
-	CHECK(params->default_action_name, EINVAL);
+	CHECK_NAME(params->default_action_name, EINVAL);
 	for (i = 0; i < p->n_actions; i++)
 		if (!strcmp(params->action_names[i],
 			    params->default_action_name))
@@ -6463,6 +6472,9 @@  rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 	      !params->default_action_data, EINVAL);
 
 	/* Table type checks. */
+	if (recommended_table_type_name)
+		CHECK_NAME(recommended_table_type_name, EINVAL);
+
 	if (params->n_fields) {
 		enum rte_swx_table_match_type match_type;
 
diff --git a/lib/librte_pipeline/rte_swx_pipeline.h b/lib/librte_pipeline/rte_swx_pipeline.h
index aed627393..f21a99556 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.h
+++ b/lib/librte_pipeline/rte_swx_pipeline.h
@@ -26,6 +26,17 @@  extern "C" {
 #ifndef RTE_SWX_NAME_SIZE
 #define RTE_SWX_NAME_SIZE 64
 #endif
+
+/** Instruction size. */
+#ifndef RTE_SWX_INSTRUCTION_SIZE
+#define RTE_SWX_INSTRUCTION_SIZE 256
+#endif
+
+/** Instruction tokens. */
+#ifndef RTE_SWX_INSTRUCTION_TOKENS_MAX
+#define RTE_SWX_INSTRUCTION_TOKENS_MAX 16
+#endif
+
 /*
  * Pipeline setup and operation
  */
diff --git a/lib/librte_pipeline/rte_swx_pipeline_spec.c b/lib/librte_pipeline/rte_swx_pipeline_spec.c
index a4bc8226a..f7884491b 100644
--- a/lib/librte_pipeline/rte_swx_pipeline_spec.c
+++ b/lib/librte_pipeline/rte_swx_pipeline_spec.c
@@ -10,9 +10,8 @@ 
 #include "rte_swx_pipeline.h"
 #include "rte_swx_ctl.h"
 
-#define MAX_LINE_LENGTH 256
-#define MAX_TOKENS 16
-#define MAX_INSTRUCTION_LENGTH 256
+#define MAX_LINE_LENGTH RTE_SWX_INSTRUCTION_SIZE
+#define MAX_TOKENS RTE_SWX_INSTRUCTION_TOKENS_MAX
 
 #define STRUCT_BLOCK 0
 #define ACTION_BLOCK 1
@@ -442,7 +441,7 @@  action_block_parse(struct action_spec *s,
 		   uint32_t *err_line,
 		   const char **err_msg)
 {
-	char buffer[MAX_INSTRUCTION_LENGTH], *instr;
+	char buffer[RTE_SWX_INSTRUCTION_SIZE], *instr;
 	const char **new_instructions;
 	uint32_t i;
 
@@ -1006,7 +1005,7 @@  apply_block_parse(struct apply_spec *s,
 		  uint32_t *err_line,
 		  const char **err_msg)
 {
-	char buffer[MAX_INSTRUCTION_LENGTH], *instr;
+	char buffer[RTE_SWX_INSTRUCTION_SIZE], *instr;
 	const char **new_instructions;
 	uint32_t i;
 
@@ -1126,6 +1125,17 @@  rte_swx_pipeline_build_from_spec(struct rte_swx_pipeline *p,
 				goto error;
 			}
 
+			/* Handle excessively long tokens. */
+			if (strnlen(token, RTE_SWX_NAME_SIZE) >=
+			    RTE_SWX_NAME_SIZE) {
+				if (err_line)
+					*err_line = n_lines;
+				if (err_msg)
+					*err_msg = "Token too big.";
+				status = -EINVAL;
+				goto error;
+			}
+
 			/* Save token. */
 			tokens[n_tokens] = token;
 			n_tokens++;