Hello Ferruh,

[:snip:]

*** stack smashing detected ***: terminated

The corruption occurred in `parse_int()` called from
`parse_indlst_id2ptr()`.

Inside `parse_int()` the arg parameter referenced 8 bytes of memory
while the target buffer was 4 bytes allocated on caller optimized stack:

(gdb) p *arg
$1 = { ... size = 8, ...}


Thanks Gregory, I can see the problem now.


[: Thumbs up :]

[:snip:]

BUT back to the root cause of the problem,
`parse_int()` tries to be generic and it support different size of
variables [1], but it fails on this.

`parse_int()` gets 'size' as argument, but it doesn't use parameter
value, instead overwrites it with 'size = arg->size;' and uses this
value, in this case when context provides larger variable size than what
`parse_int()` gets as parameter, the problem you observed occurs.

What do you think to use 'size' from parameter list, as it is intended,
instead of using 'arg->size'?
Or perhaps use 'buf' and 'size' from parameter if they are valid, else
get the from context/arg [2]?
I think this solves your problem, can you please verify it?

btw, 'buf' usage is a little more complex, since `parse_int()` checks
for "ctx->object != NULL" to continue, I can't really be sure about
intention there, but please check usage in `parse_port()`, it looks like
when 'buf' provided expectation is to get parsed value in the 'buf'.




[1]
 switch (size) {
 case sizeof(uint8_t):
   ...
 case sizeof(uint16_t):
   ...
 ...
 case sizeof(uint64_t):
   ...


[2]
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0d521159e97d..cd123c69265e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -10805,8 +10805,10 @@ parse_int(struct context *ctx, const struct
token *token,
                        goto error;
                return len;
        }
-       buf = (uint8_t *)ctx->object + arg->offset;
-       size = arg->size;
+       if (buf == NULL || size == 0) {
+               buf = (uint8_t *)ctx->object + arg->offset;
+               size = arg->size;
+       }
        if (u > RTE_LEN2MASK(size * CHAR_BIT, uint64_t))
                return -1;
 objmask:


About the fault root cause.
There were 2 uncoupled resources in that case: static token size and variable size passed to parse_int().
parse_int() caller must provide a buffer large enough for token size.
Otherwise parse_int() will corrupt memory outside the input buffer.

In the generic solution parse_int() caller allocates target buffer using existing knowledge about input token size.

Testpmd add_port() imitates the ARGS_ENTRY() macro that extrapolates target buffer size from RTE structure member.

Current testpmd cannot use that approach directly because indirect action references internal testpmd ID.

Testpmd indirect ID has no defined type or token that leads to indirect ID parser.

As a solution, testpmd can provide centralized parser function for all indirect IDs. The function will parse ID value and use the token as the key to indirect database search:

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index ce71818705..7fb3b61d37 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c

+typedef uint32_t indirect_id_t;
+
+static int
+parse_indirect_index(struct context *ctx, const struct token *token,
+               const char *str, unsigned int len,
+               void *buf, unsigned int size)
+{
+       indirect_id_t id;
+
+       ctx->object = &id;
+       parse_int(ctx, token, str, len, ctx->object, sizeof(id));
+
+       switch (ctx->curr) {
+       case INDIRECT_ACTION_ID2PTR:
+               /**/
+               break;
+       case INDIRECT_LIST_ACTION_ID2PTR_HANDLE:
+               /**/
+               break;
+       case INDIRECT_LIST_ACTION_ID2PTR_CONF:
+               /**/
+               break;
+       default:
+               /**/
+       }
+
+
+}
+

Regards,
Gregory

Reply via email to