Hello Ferruh,

Indirect actions list arguments parser was configured to place target
number into 64bit value, while the code provided 32bits memory.


Hi Gregory,

Can you please give more details why 'id' needs to be 64 bits, with
callstack or usecase etc?
And please describe what is the observed problem with current code?


In rte_flow.h, struct rte_flow_action_indirect_list::handle is a pointer.

Testpmd ACTION_INDIRECT_LIST_HANDLE and ACTION_INDIRECT_LIST_CONF tokens
define arguments size as uintptr_t.

On 64 bits system, defining the id variable as 32 bits value
corrupted parse_indlst_id2ptr stack.


I can't see how stack corruption can happen, can you please provide call
stack and flow command?


To reproduce the crash buildtype must be release or debugoptimized.
The crash will not reproduce with the debug builds.

Testpmd commands I use:

dpdk-testpmd -a ${PCI_ADDR},dv_flow_en=2,representor=vf0-1 -- -i

port stop all
flow configure 0 queues_number 12 queues_size 256
flow configure 1 queues_number 12 queues_size 256
flow configure 2 queues_number 12 queues_size 256
port start all
start

set raw_encap 0 eth dst is 00:16:3e:52:bd:37 src is 00:16:3e:6e:16:e0 type is 2048 has_vlan is 0 / ipv4 src is 110.240.52.255 dst is 189.68.183.147 proto is 17 fragment_offset is 0 packet_id is 1 tos is 102 ttl is 189 version_ihl is 69 / udp src is 56800 dst is 4789 / vxlan vni is 3 / end_set
set sample_actions 0 represented_port ethdev_port_id 0 / end

flow indirect_action 0 create action_id 5 transfer list actions sample ratio 1 index 0 / represented_port ethdev_port_id 2 / end flow actions_template 0 create transfer actions_template_id 6 template indirect_list handle 5 / end mask indirect_list handle 5 / end

Result:
*** 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, ...}


Inside 'parse_indlst_id2ptr()',
'parse_int()' can work or 32bits and 64bits variables, so that one is OK.
But both 'port_action_handle_get_by_id()' &
'indirect_action_list_conf_get()' gets 'id' as parameter and they get
32bits argument, when 'id' is 64bit won't it will be cast to 32bits and
loose data, should those functions needs to be updated as well.


Can you please reply to above question, about changing 'id' type impact
to other functions using it?


I've missed that.
Need to re-think.

Regards,
Gregory




The patch updated variable size for translation results.

Fixes: 72a3dec7126f ("ethdev: add indirect flow list action")
Signed-off-by: Gregory Etelson <getel...@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0d521159e9..cf1ca33208 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -11331,7 +11331,7 @@ parse_indlst_id2ptr(struct context *ctx,
const struct token *token,
      struct rte_flow_action *action = ctx->object;
      struct rte_flow_action_indirect_list *action_conf;
      const struct indlst_conf *indlst_conf;
-     uint32_t id;
+     uint64_t id;
      int ret;

      if (!action)
@@ -11350,7 +11350,8 @@ parse_indlst_id2ptr(struct context *ctx,
const struct token *token,
      action_conf->handle = (typeof(action_conf->handle))
                              port_action_handle_get_by_id(ctx->port,
id);
              if (!action_conf->handle) {
-                     printf("no indirect list handle for id %u\n", id);
+                     printf("no indirect list handle for id
%"PRIu64"\n",
+                            id);
                      return -1;
              }
              break;




Reply via email to