Re: [ovs-dev] [PATCH 2/5] odp-util: Fix use of uninitialized erspan metadata.
On 4/5/21 4:31 PM, William Tu wrote:
> On Sun, Apr 4, 2021 at 10:31 AM Ilya Maximets wrote:
>>
>> 'struct erspan_metadata' contains union with fields of different
>> sizes, hence not all the memory initiliazed. This memory goes
>> to syscalls and also used to compare ukeys with memcmp which may
>> cause unexpected behavior.
>>
>> Thread 15 revalidator13:
>> Conditional jump or move depends on uninitialised value(s)
>> at 0x4C377B6: bcmp (vg_replace_strmem.c:)
>> by 0x43F844: ofpbuf_equal (ofpbuf.h:273)
>> by 0x43F844: revalidate_ukey__ (ofproto-dpif-upcall.c:2227)
>> by 0x43F9C9: revalidate_ukey (ofproto-dpif-upcall.c:2294)
>> by 0x4425C2: revalidate.isra.33 (ofproto-dpif-upcall.c:2734)
>> by 0x4434B8: udpif_revalidator (ofproto-dpif-upcall.c:943)
>> by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
>> by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
>> by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
>> Uninitialised value was created by a stack allocation
>> at 0x4B1CE0: tun_key_to_attr (odp-util.c:3129)
>>
>> CC: William Tu
>> Fixes: 98514eea21f4 ("erspan: add kernel datapath support")
>> Signed-off-by: Ilya Maximets
>> ---
>
> LGTM, Thanks.
> Acked-by: William Tu
>
Thanks! Applied to master and backported down to 2.12.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/5] odp-util: Fix use of uninitialized erspan metadata.
On Sun, Apr 4, 2021 at 10:31 AM Ilya Maximets wrote:
>
> 'struct erspan_metadata' contains union with fields of different
> sizes, hence not all the memory initiliazed. This memory goes
> to syscalls and also used to compare ukeys with memcmp which may
> cause unexpected behavior.
>
> Thread 15 revalidator13:
> Conditional jump or move depends on uninitialised value(s)
> at 0x4C377B6: bcmp (vg_replace_strmem.c:)
> by 0x43F844: ofpbuf_equal (ofpbuf.h:273)
> by 0x43F844: revalidate_ukey__ (ofproto-dpif-upcall.c:2227)
> by 0x43F9C9: revalidate_ukey (ofproto-dpif-upcall.c:2294)
> by 0x4425C2: revalidate.isra.33 (ofproto-dpif-upcall.c:2734)
> by 0x4434B8: udpif_revalidator (ofproto-dpif-upcall.c:943)
> by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
> by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
> by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
> Uninitialised value was created by a stack allocation
> at 0x4B1CE0: tun_key_to_attr (odp-util.c:3129)
>
> CC: William Tu
> Fixes: 98514eea21f4 ("erspan: add kernel datapath support")
> Signed-off-by: Ilya Maximets
> ---
LGTM, Thanks.
Acked-by: William Tu
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/5] odp-util: Fix use of uninitialized erspan metadata.
'struct erspan_metadata' contains union with fields of different
sizes, hence not all the memory initiliazed. This memory goes
to syscalls and also used to compare ukeys with memcmp which may
cause unexpected behavior.
Thread 15 revalidator13:
Conditional jump or move depends on uninitialised value(s)
at 0x4C377B6: bcmp (vg_replace_strmem.c:)
by 0x43F844: ofpbuf_equal (ofpbuf.h:273)
by 0x43F844: revalidate_ukey__ (ofproto-dpif-upcall.c:2227)
by 0x43F9C9: revalidate_ukey (ofproto-dpif-upcall.c:2294)
by 0x4425C2: revalidate.isra.33 (ofproto-dpif-upcall.c:2734)
by 0x4434B8: udpif_revalidator (ofproto-dpif-upcall.c:943)
by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
Uninitialised value was created by a stack allocation
at 0x4B1CE0: tun_key_to_attr (odp-util.c:3129)
CC: William Tu
Fixes: 98514eea21f4 ("erspan: add kernel datapath support")
Signed-off-by: Ilya Maximets
---
lib/odp-util.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index a8598d52a..e1199d1da 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3189,17 +3189,17 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl
*tun_key,
if ((!tnl_type || !strcmp(tnl_type, "erspan") ||
!strcmp(tnl_type, "ip6erspan")) &&
(tun_key->erspan_ver == 1 || tun_key->erspan_ver == 2)) {
-struct erspan_metadata opts;
+struct erspan_metadata *opts;
-opts.version = tun_key->erspan_ver;
-if (opts.version == 1) {
-opts.u.index = htonl(tun_key->erspan_idx);
+opts = nl_msg_put_unspec_zero(a, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,
+ sizeof *opts);
+opts->version = tun_key->erspan_ver;
+if (opts->version == 1) {
+opts->u.index = htonl(tun_key->erspan_idx);
} else {
-opts.u.md2.dir = tun_key->erspan_dir;
-set_hwid(&opts.u.md2, tun_key->erspan_hwid);
+opts->u.md2.dir = tun_key->erspan_dir;
+set_hwid(&opts->u.md2, tun_key->erspan_hwid);
}
-nl_msg_put_unspec(a, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,
- &opts, sizeof(opts));
}
if ((!tnl_type || !strcmp(tnl_type, "gtpu")) &&
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
