diff options
| author | Ilya Maximets <i.maximets@ovn.org> | 2020-01-23 19:10:05 +0100 |
|---|---|---|
| committer | Ilya Maximets <i.maximets@ovn.org> | 2020-01-27 21:20:01 +0100 |
| commit | 342b8904ab4f29b2a4a429e032f30ddad420a29e (patch) | |
| tree | 1ade64cc5d1061535908d5c20324fbaf6f90e102 /ofproto/ofproto-dpif.c | |
| parent | 79eadafeb1b47a3871cb792aa972f6e4d89d1a0b (diff) | |
| download | openvswitch-342b8904ab4f29b2a4a429e032f30ddad420a29e.tar.gz | |
dpif: Fix dp_extra_info leak by reworking the allocation scheme.
dpctl module leaks the 'dp_extra_info' in case the dumped flow doesn't
fit the dump filter while executing dpctl/dump-flows and also while
executing dpctl/get-flow.
This is already a 3rd attempt to fix all the leaks and incorrect usage
of this string that definitely indicates poor initial design of the
feature.
Flow dump/get documentation clearly states that the caller does not own
the data provided in dpif_flow. Datapath still owns all the data and
promises to not free/modify it until the next quiescent period, however
we're requesting the caller to free 'dp_extra_info' and this obviously
breaks the rules.
This patch fixes the issue by by storing 'dp_extra_info' within
'struct dp_netdev_flow' making datapath to own it. 'dp_netdev_flow'
is RCU-protected, so it will be valid until the next quiescent period.
Fixes: 0e8f5c6a38d0 ("dpif-netdev: Modified ovs-appctl dpctl/dump-flows command")
Tested-by: Emma Finn <emma.finn@intel.com>
Acked-by: Emma Finn <emma.finn@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'ofproto/ofproto-dpif.c')
| -rw-r--r-- | ofproto/ofproto-dpif.c | 3 |
1 files changed, 0 insertions, 3 deletions
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 67a4ad46f..0222ec82f 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -6278,9 +6278,6 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn, while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) { struct flow flow; - /* No need for extra info. */ - free(f.attrs.dp_extra_info); - if ((odp_flow_key_to_flow(f.key, f.key_len, &flow, NULL) == ODP_FIT_ERROR) || (xlate_lookup_ofproto(ofproto->backer, &flow, NULL, NULL) |