* [dpdk-dev] [PATCH] pipeline: fix string copy into fixed size buffer
@ 2020-10-26 19:59 Cristian Dumitrescu
2020-10-29 16:31 ` David Marchand
0 siblings, 1 reply; 2+ messages in thread
From: Cristian Dumitrescu @ 2020-10-26 19:59 UTC (permalink / raw)
To: dev
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(-)
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++;
--
2.17.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [dpdk-dev] [PATCH] pipeline: fix string copy into fixed size buffer
2020-10-26 19:59 [dpdk-dev] [PATCH] pipeline: fix string copy into fixed size buffer Cristian Dumitrescu
@ 2020-10-29 16:31 ` David Marchand
0 siblings, 0 replies; 2+ messages in thread
From: David Marchand @ 2020-10-29 16:31 UTC (permalink / raw)
To: Cristian Dumitrescu; +Cc: dev
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.
--
David Marchand
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-29 16:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 19:59 [dpdk-dev] [PATCH] pipeline: fix string copy into fixed size buffer Cristian Dumitrescu
2020-10-29 16:31 ` David Marchand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).