Hello Ferruh,

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.


As you said 'parse_int()' has two sizes, 'token->size' and 'size'
function argument. Why function ignores 'size' argument and only uses
'token->size', I think this is a mistake.


If 'parse_int()' doesn't use 'buf' and 'size' arguments at all, why it
has them?



parse_int() receives target buffer size as an argument and also has access to original token size through the arg pointer.
In that configuration the token size has priority.

parse_int() must compare token and buffer sizes and return an error if target buffer size did not fit.

That will not break existing parser_int() functionality.
I'll send a fix in a new patch series.

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:


Although it sounds reasonable to have indirect id parser, won't it have
exact same problem?

If token size if 64 bits as it is now, as far as I can see below code
will have same stack corruption problem.


The function parses indirect ID. That ID is translated into the original object of any size according to token type. That means testpmd will have common pool of 32 bits IDs for all possible indirect translations.


I think we should update parse_int function, to use either function
parameters or context values, but changes has potential side effect and
timing is not good for it, let's continue with your v3 for now.



[: Thumbs up :]

Regards,
Gregory

Reply via email to