]> git.proxmox.com Git - mirror_ovs.git/commit
ofproto-dpif: Fix using uninitialised memory in user_action_cookie.
authorIlya Maximets <i.maximets@samsung.com>
Thu, 25 Jul 2019 15:11:13 +0000 (18:11 +0300)
committerIlya Maximets <i.maximets@samsung.com>
Fri, 23 Aug 2019 14:23:31 +0000 (17:23 +0300)
commit24a4bbe1de11eb3bb2da0c5ea0e1fa41e730600c
treeed33fcc7cd665107663da65fdd7254a6a41520a3
parentd578e064c5ce92d3ded8d36675b34866e0989eea
ofproto-dpif: Fix using uninitialised memory in user_action_cookie.

Designated initializers are not suitable for initializing non-packed
structures and unions which are subjects for comparison by memcmp().

Whole memory for 'struct user_action_cookie' must be explicitly cleared
before using because it will be copied with memcpy and later compared
by memcmp in ofpbuf_equal().

Few issues found be valgrind:

 Thread 13 revalidator11:
 Conditional jump or move depends on uninitialised value(s)
    at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
    by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
    by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219)
    by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2286)
    by 0x9D62AC: revalidate (ofproto-dpif-upcall.c:2685)
    by 0x9D62AC: udpif_revalidator (ofproto-dpif-upcall.c:942)
    by 0xA9C732: ovsthread_wrapper (ovs-thread.c:383)
    by 0x5FF86DA: start_thread (pthread_create.c:463)
    by 0x6AF488E: clone (clone.S:95)
  Uninitialised value was created by a stack allocation
    at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062)

 Thread 11 revalidator16:
 Conditional jump or move depends on uninitialised value(s)
    at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
    by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
    by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2220)
    by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2287)
    by 0x9D62BC: revalidate (ofproto-dpif-upcall.c:2686)
    by 0x9D62BC: udpif_revalidator (ofproto-dpif-upcall.c:942)
    by 0xA9C6D2: ovsthread_wrapper (ovs-thread.c:383)
    by 0x5FF86DA: start_thread (pthread_create.c:463)
    by 0x6AF488E: clone (clone.S:95)
  Uninitialised value was created by a stack allocation
    at 0x9DC4E0: compose_sflow_action (ofproto-dpif-xlate.c:3211)

The struct was never marked as 'packed', however it was manually
adjusted to be so in practice.
Old IPFIX related commit first made the structure non-contiguous.
Commit 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.")
added uninitialized parts of the additional union space and the next
one introduced new holes between structure fields for all cases.

CC: Justin Pettit <jpettit@ovn.org>
Fixes: 8b7ea2d48033 ("Extend OVS IPFIX exporter to export tunnel headers")
Fixes: 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.")
Fixes: fcb9579be3c7 ("ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Ben Pfaff <blp@ovn.org>
ofproto/ofproto-dpif-upcall.c
ofproto/ofproto-dpif-xlate.c