Pointers to gnet_stats_basic and tcf_t are not correctly aligned,
so we need to copy the data before accessing. Found by running
check-offloads testsuite with UBsan:
lib/tc.c:1791:50: runtime error:
member access within misaligned address 0x61900005ce1c for type
'const struct gnet_stats_basic', which requires 8 byte alignment
0x61900005ce1c: note: pointer points here
14 00 07 00 00 00 00 00 00 00 00 00 00 00 00 00 ...
^
0 0xd69044 in nl_parse_single_action lib/tc.c:1791:50
1 0xd69044 in nl_parse_flower_actions lib/tc.c:1841:19
2 0xd57612 in nl_parse_flower_options lib/tc.c:1893:12
3 0xd5468d in parse_netlink_to_tc_flower lib/tc.c:1941:12
4 0xd5a7ec in tc_replace_flower lib/tc.c:3199:19
5 0xd2baea in probe_tc_block_support lib/netdev-offload-tc.c:2226:13
6 0xd2baea in netdev_tc_init_flow_api lib/netdev-offload-tc.c:2279:9
7 0x94d726 in netdev_assign_flow_api lib/netdev-offload.c:183:14
8 0x94d726 in netdev_init_flow_api lib/netdev-offload.c:323:9
9 0x9515c7 in netdev_ports_flow_init lib/netdev-offload.c:775:8
10 0x9515c7 in netdev_set_flow_api_enabled lib/netdev-offload.c:815:13
11 0x562ec8 in bridge_run vswitchd/bridge.c:3292:9
12 0x59a98c in main vswitchd/ovs-vswitchd.c:129:9
13 0x7fb5c583acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2)
14 0x47e60d in _start (vswitchd/ovs-vswitchd+0x47e60d)
lib/tc.c:1306:43: runtime error:
member access within misaligned address 0x619000140324 for type
'const struct tcf_t', which requires 8 byte alignment
0x619000140324: note: pointer points here
24 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 ...
^
0 0xd6ee6f in nl_parse_tcf lib/tc.c:1306:43
1 0xd6423f in nl_parse_act_mirred lib/tc.c:1389:5
2 0xd6423f in nl_parse_single_action lib/tc.c:1747:15
3 0xd6423f in nl_parse_flower_actions lib/tc.c:1843:19
4 0xd57612 in nl_parse_flower_options lib/tc.c:1895:12
5 0xd5468d in parse_netlink_to_tc_flower lib/tc.c:1943:12
6 0xd5a7ec in tc_replace_flower lib/tc.c:3201:19
7 0xd28ae8 in netdev_tc_flow_put lib/netdev-offload-tc.c:1969:11
8 0x94cc97 in netdev_flow_put lib/netdev-offload.c:257:14
9 0xcba2be in parse_flow_put lib/dpif-netlink.c:2289:11
10 0xcba2be in try_send_to_netdev lib/dpif-netlink.c:2376:15
11 0xcba2be in dpif_netlink_operate lib/dpif-netlink.c:2447:23
12 0x8647ce in dpif_operate lib/dpif.c:1372:13
13 0x6b50a6 in push_dp_ops ofproto/ofproto-dpif-upcall.c:2406:5
14 0x6c9bcd in revalidate ofproto/ofproto-dpif-upcall.c:2792:13
15 0x6b79b5 in udpif_revalidator ofproto/ofproto-dpif-upcall.c:980:9
16 0xb4d5ea in ovsthread_wrapper lib/ovs-thread.c:422:12
17 0x7ff1090081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
18 0x7ff107c39dd2 in clone (/lib64/libc.so.6+0x39dd2)
This patch fixes only problems reported by UBsan in current tests,
there could be more issues like this not currently covered by the
testsuite.
Signed-off-by: Ilya Maximets <[email protected]>
---
lib/tc.c | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/lib/tc.c b/lib/tc.c
index df73a43d4..bbb8c86f7 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1312,8 +1312,8 @@ nl_parse_act_gact(struct nlattr *options, struct
tc_flower *flower)
struct nlattr *gact_attrs[ARRAY_SIZE(gact_policy)];
const struct tc_gact *p;
struct nlattr *gact_parms;
- const struct tcf_t *tm;
struct tc_action *action;
+ struct tcf_t tm;
if (!nl_parse_nested(options, gact_policy, gact_attrs,
ARRAY_SIZE(gact_policy))) {
@@ -1333,8 +1333,9 @@ nl_parse_act_gact(struct nlattr *options, struct
tc_flower *flower)
return EINVAL;
}
- tm = nl_attr_get_unspec(gact_attrs[TCA_GACT_TM], sizeof *tm);
- nl_parse_tcf(tm, flower);
+ memcpy(&tm, nl_attr_get_unspec(gact_attrs[TCA_GACT_TM], sizeof tm),
+ sizeof tm);
+ nl_parse_tcf(&tm, flower);
return 0;
}
@@ -1355,9 +1356,9 @@ nl_parse_act_mirred(struct nlattr *options, struct
tc_flower *flower)
struct nlattr *mirred_attrs[ARRAY_SIZE(mirred_policy)];
const struct tc_mirred *m;
const struct nlattr *mirred_parms;
- const struct tcf_t *tm;
struct nlattr *mirred_tm;
struct tc_action *action;
+ struct tcf_t tm;
if (!nl_parse_nested(options, mirred_policy, mirred_attrs,
ARRAY_SIZE(mirred_policy))) {
@@ -1385,8 +1386,8 @@ nl_parse_act_mirred(struct nlattr *options, struct
tc_flower *flower)
action->type = TC_ACT_OUTPUT;
mirred_tm = mirred_attrs[TCA_MIRRED_TM];
- tm = nl_attr_get_unspec(mirred_tm, sizeof *tm);
- nl_parse_tcf(tm, flower);
+ memcpy(&tm, nl_attr_get_unspec(mirred_tm, sizeof tm), sizeof tm);
+ nl_parse_tcf(&tm, flower);
return 0;
}
@@ -1725,9 +1726,7 @@ nl_parse_single_action(struct nlattr *action, struct
tc_flower *flower,
struct nlattr *stats_attrs[ARRAY_SIZE(stats_policy)];
struct ovs_flow_stats *stats_sw = &flower->stats_sw;
struct ovs_flow_stats *stats_hw = &flower->stats_hw;
- const struct gnet_stats_basic *bs_all = NULL;
- const struct gnet_stats_basic *bs_hw = NULL;
- struct gnet_stats_basic bs_sw = { .packets = 0, .bytes = 0, };
+ struct gnet_stats_basic bs_all, bs_hw, bs_sw;
int err = 0;
if (!nl_parse_nested(action, act_policy, action_attrs,
@@ -1783,16 +1782,19 @@ nl_parse_single_action(struct nlattr *action, struct
tc_flower *flower,
return EPROTO;
}
- bs_all = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs_all);
+ memcpy(&bs_all,
+ nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof bs_all),
+ sizeof bs_all);
if (stats_attrs[TCA_STATS_BASIC_HW]) {
- bs_hw = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC_HW],
- sizeof *bs_hw);
+ memcpy(&bs_hw, nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC_HW],
+ sizeof bs_hw),
+ sizeof bs_hw);
- bs_sw.packets = bs_all->packets - bs_hw->packets;
- bs_sw.bytes = bs_all->bytes - bs_hw->bytes;
+ bs_sw.packets = bs_all.packets - bs_hw.packets;
+ bs_sw.bytes = bs_all.bytes - bs_hw.bytes;
} else {
- bs_sw.packets = bs_all->packets;
- bs_sw.bytes = bs_all->bytes;
+ bs_sw.packets = bs_all.packets;
+ bs_sw.bytes = bs_all.bytes;
}
if (bs_sw.packets > get_32aligned_u64(&stats_sw->n_packets)) {
@@ -1800,9 +1802,10 @@ nl_parse_single_action(struct nlattr *action, struct
tc_flower *flower,
put_32aligned_u64(&stats_sw->n_bytes, bs_sw.bytes);
}
- if (bs_hw && bs_hw->packets > get_32aligned_u64(&stats_hw->n_packets)) {
- put_32aligned_u64(&stats_hw->n_packets, bs_hw->packets);
- put_32aligned_u64(&stats_hw->n_bytes, bs_hw->bytes);
+ if (stats_attrs[TCA_STATS_BASIC_HW]
+ && bs_hw.packets > get_32aligned_u64(&stats_hw->n_packets)) {
+ put_32aligned_u64(&stats_hw->n_packets, bs_hw.packets);
+ put_32aligned_u64(&stats_hw->n_bytes, bs_hw.bytes);
}
return 0;
--
2.34.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
