mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-30 22:05:19 +00:00
Code Issues Releases Wiki Activity

ofproto-dpif-trace: Fix access to an out-of-scope stack memory.

Browse Source 
While tracing NAT actions, pointer to the action may be stored in the
recirculation node for future reference.  However, while translating
actions for the group bucket in xlate_group_bucket, the action list is
allocated temporarily on stack.  So, in case the group translation
leads to NAT, the stack pointer can be stored in the recirculation node
and accessed later by the tracing mechanism when this stack memory is
long gone:

 ==396230==ERROR: AddressSanitizer: stack-use-after-return on address
 0x191844 at pc 0x64222a bp 0xa5da10 sp 0xa5da08
 READ of size 1 at 0x191844 thread T0
  0 0x642229 in ofproto_trace_recirc_node ofproto/ofproto-dpif-trace.c:704:49
  1 0x642229 in ofproto_trace ofproto/ofproto-dpif-trace.c:867:9
  2 0x6434c1 in ofproto_unixctl_trace ofproto/ofproto-dpif-trace.c:489:9
  3 0xc1e491 in process_command lib/unixctl.c:310:13
  4 0xc1e491 in run_connection lib/unixctl.c:344:17
  5 0xc1e491 in unixctl_server_run lib/unixctl.c:395:21
  6 0x53eedf in main ovs/vswitchd/ovs-vswitchd.c:131:9
  7 0x2be087 in __libc_start_call_main
  8 0x2be14a in __libc_start_main@GLIBC_2.2.5
  9 0x42dee4 in _start (vswitchd/ovs-vswitchd+0x42dee4)

 Address 0x191844 is located in stack of thread T0 at offset 68 in frame
  0 0x6d391f in xlate_group_bucket ofproto/ofproto-dpif-xlate.c:4751

  This frame has 3 object(s):
    [32, 1056) 'action_list_stub' (line 4760) <== Memory access at
                                                  offset 68 is inside
                                                  this variable
    [1184, 1248) 'action_list' (line 4761)
    [1280, 1344) 'action_set' (line 4762)

 SUMMARY: AddressSanitizer: stack-use-after-return
   ofproto/ofproto-dpif-trace.c:704:49 in ofproto_trace_recirc_node

Fix that by copying the action.

Fixes: d072d2de01 ("ofproto-dpif-trace: Improve NAT tracing.")
Reported-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Ilya Maximets
2024-05-03 01:36:37 +02:00
parent f0e0e48ec5
commit b91f6788c4
3 changed files with 25 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
ofproto/ofproto-dpif-trace.c
View File

@@ -102,7 +102,7 @@ oftrace_add_recirc_node(struct ovs_list *recirc_queue,
node->flow = *flow;
node->flow.recirc_id = recirc_id;
node->flow.ct_zone = zone;
node->nat_act = ofn;
node->nat_act = ofn ? xmemdup(ofn, sizeof *ofn) : NULL;
node->packet = packet ? dp_packet_clone(packet) : NULL;
return true;
@@ -113,6 +113,7 @@ oftrace_recirc_node_destroy(struct oftrace_recirc_node *node)
{
if (node) {
recirc_free_id(node->recirc_id);
free(node->nat_act);
dp_packet_delete(node->packet);
free(node);
}

2
ofproto/ofproto-dpif-trace.h
View File

@@ -73,7 +73,7 @@ struct oftrace_recirc_node {
uint32_t recirc_id;
struct flow flow;
struct dp_packet *packet;
const struct ofpact_nat *nat_act;
struct ofpact_nat *nat_act;
};
/* A node within a next_ct_states list. */

22
tests/ofproto-dpif.at
View File

@@ -947,6 +947,28 @@ AT_CHECK([tail -1 stdout], [0],
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ofproto-dpif - group with ct and dnat recirculation in action list])
OVS_VSWITCHD_START
add_of_ports br0 1 10
AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 \
'group_id=1234,type=all,bucket=ct(nat(dst=10.10.10.7:80),commit,table=2)'])
AT_DATA([flows.txt], [dnl
table=0 ip,ct_state=-trk actions=group:1234
table=2 ip,ct_state=+trk actions=output:10
])
AT_CHECK([ovs-ofctl -O OpenFlow12 add-flows br0 flows.txt])
AT_CHECK([ovs-appctl ofproto/trace br0 '
in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,
nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,nw_frag=no,
icmp_type=8,icmp_code=0
'], [0], [stdout])
AT_CHECK([grep 'Datapath actions' stdout], [0], [dnl
Datapath actions: ct(commit,nat(dst=10.10.10.7:80)),recirc(0x1)
Datapath actions: 10
])
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ofproto-dpif - group actions have no effect afterwards])
OVS_VSWITCHD_START
add_of_ports br0 1 10