Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
Hi, Ben Pfaff Thank you. Give a like. > -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Tuesday, December 3, 2019 4:21 AM > To: Numan Siddique > Cc: Linhaifeng ; d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow > > On Fri, Nov 29, 2019 at 04:15:59PM +0530, Numan Siddique wrote: > > On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng > wrote: > > > > > > Should use flow->actions not >actions. > > > > > > here is ASAN report: > > > > > = > > > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address > 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ > of size 196 at 0x428fa0e8 thread T150 (revalidator22) > > > #0 0x7f61a51f in __interceptor_memcpy > (/lib64/libasan.so.4+0xa251f) > > > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426 > > > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom > lib/ofpbuf.c:248 > > > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218 > > > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208 > > > #5 0xd23e3993 in ukey_set_actions > ofproto/ofproto-dpif-upcall.c:1640 > > > #6 0xd23e3f03 in ukey_create__ > ofproto/ofproto-dpif-upcall.c:1696 > > > #7 0xd23e553f in ukey_create_from_dpif_flow > ofproto/ofproto-dpif-upcall.c:1806 > > > #8 0xd23e65fb in ukey_acquire > ofproto/ofproto-dpif-upcall.c:1984 > > > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625 > > > #10 0xd23dee5f in udpif_revalidator > ofproto/ofproto-dpif-upcall.c:1076 > > > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708 > > > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb) > > > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb) > > > > > > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) > > > at > offset 328 in frame > > > #0 0xd23e4cab in ukey_create_from_dpif_flow > > > ofproto/ofproto-dpif-upcall.c:1762 > > > > > > This frame has 4 object(s): > > > [32, 96) 'actions' > > > [128, 192) 'buf' > > > [224, 328) 'full_flow' > > > [384, 2432) 'stub' <== Memory access at offset 328 partially > > > underflows this variable > > > HINT: this may be a false positive if your program uses some custom stack > unwind mechanism or swapcontext > > > (longjmp and C++ exceptions *are* supported) Thread T150 > (revalidator22) created by T0 here: > > > #0 0x7f5b0f7f in __interceptor_pthread_create > (/lib64/libasan.so.4+0x38f7f) > > > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792 > > > #2 0xd23dc62f in udpif_start_threads > ofproto/ofproto-dpif-upcall.c:639 > > > #3 0xd23daf87 in ofproto_set_flow_table > ofproto/ofproto-dpif-upcall.c:446 > > > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134 > > > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148 > > > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944 > > > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240 > > > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf) > > > #9 0xd230a3d3 > > > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3) > > > > > > SUMMARY: AddressSanitizer: stack-buffer-overflow > (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around > the buggy address: > > > 0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00 > > > 0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 > > > 0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 > > > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 > > > 0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > 0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Shadow byte legend (one shadow byte represents 8 application bytes): > > > Addressabl
Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
On Fri, Nov 29, 2019 at 04:15:59PM +0530, Numan Siddique wrote: > On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng wrote: > > > > Should use flow->actions not >actions. > > > > here is ASAN report: > > = > > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address > > 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 > > READ of size 196 at 0x428fa0e8 thread T150 (revalidator22) > > #0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f) > > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426 > > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248 > > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218 > > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208 > > #5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640 > > #6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696 > > #7 0xd23e553f in ukey_create_from_dpif_flow > > ofproto/ofproto-dpif-upcall.c:1806 > > #8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984 > > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625 > > #10 0xd23dee5f in udpif_revalidator > > ofproto/ofproto-dpif-upcall.c:1076 > > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708 > > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb) > > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb) > > > > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) > > at offset 328 in frame > > #0 0xd23e4cab in ukey_create_from_dpif_flow > > ofproto/ofproto-dpif-upcall.c:1762 > > > > This frame has 4 object(s): > > [32, 96) 'actions' > > [128, 192) 'buf' > > [224, 328) 'full_flow' > > [384, 2432) 'stub' <== Memory access at offset 328 partially underflows > > this variable > > HINT: this may be a false positive if your program uses some custom stack > > unwind mechanism or swapcontext > > (longjmp and C++ exceptions *are* supported) Thread T150 > > (revalidator22) created by T0 here: > > #0 0x7f5b0f7f in __interceptor_pthread_create > > (/lib64/libasan.so.4+0x38f7f) > > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792 > > #2 0xd23dc62f in udpif_start_threads > > ofproto/ofproto-dpif-upcall.c:639 > > #3 0xd23daf87 in ofproto_set_flow_table > > ofproto/ofproto-dpif-upcall.c:446 > > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134 > > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148 > > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944 > > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240 > > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf) > > #9 0xd230a3d3 > > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3) > > > > SUMMARY: AddressSanitizer: stack-buffer-overflow > > (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around > > the buggy address: > > 0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00 > > 0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 > > 0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 > > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 > > 0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow > > byte legend (one shadow byte represents 8 application bytes): > > Addressable: 00 > > Partially addressable: 01 02 03 04 05 06 07 > > Heap left redzone: fa > > Freed heap region: fd > > Stack left redzone: f1 > > Stack mid redzone: f2 > > Stack right redzone: f3 > > Stack after return: f5 > > Stack use after scope: f8 > > Global redzone: f9 > > Global init order: f6 > > Poisoned by user:f7 > > Container overflow: fc > > Array cookie:ac > > Intra object redzone:bb > > ASan internal: fe > > Left alloca redzone: ca > > Right alloca redzone:cb > > ==57189==ABORTING > > > > Signed-off-by: Linhaifeng > > Acked-by: Numan Siddique Linhaifeng, this is a great catch. Thank you. I applied this to master and backported it. I also checked through the source tree for other, similar issues. I did not find any. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng wrote: > > Should use flow->actions not >actions. > > here is ASAN report: > = > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address > 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ > of size 196 at 0x428fa0e8 thread T150 (revalidator22) > #0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f) > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426 > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248 > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218 > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208 > #5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640 > #6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696 > #7 0xd23e553f in ukey_create_from_dpif_flow > ofproto/ofproto-dpif-upcall.c:1806 > #8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984 > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625 > #10 0xd23dee5f in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1076 > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708 > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb) > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb) > > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) at > offset 328 in frame > #0 0xd23e4cab in ukey_create_from_dpif_flow > ofproto/ofproto-dpif-upcall.c:1762 > > This frame has 4 object(s): > [32, 96) 'actions' > [128, 192) 'buf' > [224, 328) 'full_flow' > [384, 2432) 'stub' <== Memory access at offset 328 partially underflows > this variable > HINT: this may be a false positive if your program uses some custom stack > unwind mechanism or swapcontext > (longjmp and C++ exceptions *are* supported) Thread T150 > (revalidator22) created by T0 here: > #0 0x7f5b0f7f in __interceptor_pthread_create > (/lib64/libasan.so.4+0x38f7f) > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792 > #2 0xd23dc62f in udpif_start_threads ofproto/ofproto-dpif-upcall.c:639 > #3 0xd23daf87 in ofproto_set_flow_table > ofproto/ofproto-dpif-upcall.c:446 > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134 > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148 > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944 > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240 > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf) > #9 0xd230a3d3 > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3) > > SUMMARY: AddressSanitizer: stack-buffer-overflow > (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around the > buggy address: > 0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00 > 0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 > 0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 > 0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte > legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Freed heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user:f7 > Container overflow: fc > Array cookie:ac > Intra object redzone:bb > ASan internal: fe > Left alloca redzone: ca > Right alloca redzone:cb > ==57189==ABORTING > > Signed-off-by: Linhaifeng Acked-by: Numan Siddique Numan > --- > ofproto/ofproto-dpif-upcall.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index dc30824771..c2fc527a31 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -1796,7 +1796,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif, > } > > reval_seq = seq_read(udpif->reval_seq) - 1; /* Ensure revalidation. */ > -ofpbuf_use_const(, >actions, flow->actions_len); > +ofpbuf_use_const(, flow->actions, flow->actions_len); >
[ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
Should use flow->actions not >actions. here is ASAN report: = ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ of size 196 at 0x428fa0e8 thread T150 (revalidator22) #0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f) #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426 #2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248 #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218 #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208 #5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640 #6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696 #7 0xd23e553f in ukey_create_from_dpif_flow ofproto/ofproto-dpif-upcall.c:1806 #8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984 #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625 #10 0xd23dee5f in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1076 #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708 #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb) #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb) Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) at offset 328 in frame #0 0xd23e4cab in ukey_create_from_dpif_flow ofproto/ofproto-dpif-upcall.c:1762 This frame has 4 object(s): [32, 96) 'actions' [128, 192) 'buf' [224, 328) 'full_flow' [384, 2432) 'stub' <== Memory access at offset 328 partially underflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) Thread T150 (revalidator22) created by T0 here: #0 0x7f5b0f7f in __interceptor_pthread_create (/lib64/libasan.so.4+0x38f7f) #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792 #2 0xd23dc62f in udpif_start_threads ofproto/ofproto-dpif-upcall.c:639 #3 0xd23daf87 in ofproto_set_flow_table ofproto/ofproto-dpif-upcall.c:446 #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134 #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148 #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944 #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240 #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf) #9 0xd230a3d3 (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3) SUMMARY: AddressSanitizer: stack-buffer-overflow (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around the buggy address: 0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00 0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user:f7 Container overflow: fc Array cookie:ac Intra object redzone:bb ASan internal: fe Left alloca redzone: ca Right alloca redzone:cb ==57189==ABORTING Signed-off-by: Linhaifeng --- ofproto/ofproto-dpif-upcall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index dc30824771..c2fc527a31 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1796,7 +1796,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif, } reval_seq = seq_read(udpif->reval_seq) - 1; /* Ensure revalidation. */ -ofpbuf_use_const(, >actions, flow->actions_len); +ofpbuf_use_const(, flow->actions, flow->actions_len); *ukey = ukey_create__(flow->key, flow->key_len, flow->mask, flow->mask_len, flow->ufid_present, >ufid, flow->pmd_id, , -- 2.19.1 ___ dev mailing list d...@openvswitch.org
Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
Bleep bloop. Greetings Linhaifeng, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author Linhaifeng needs to sign off. Lines checked: 104, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
Should use flow->actions not >actions. Here is some print info for debug: @@@ flow->actions=0xb5e48844 len=196 @@@ stub=[0x7eefa120,0x7eefa918] actions->data=0x7eefa0a0 actions->size=196 actions->data != flow->actions and actions->data + 196 is in the range of stub . here is ASAN report: = ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ of size 196 at 0x428fa0e8 thread T150 (revalidator22) #0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f) #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426 #2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248 #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218 #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208 #5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640 #6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696 #7 0xd23e553f in ukey_create_from_dpif_flow ofproto/ofproto-dpif-upcall.c:1806 #8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984 #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625 #10 0xd23dee5f in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1076 #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708 #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb) #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb) Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) at offset 328 in frame #0 0xd23e4cab in ukey_create_from_dpif_flow ofproto/ofproto-dpif-upcall.c:1762 This frame has 4 object(s): [32, 96) 'actions' [128, 192) 'buf' [224, 328) 'full_flow' [384, 2432) 'stub' <== Memory access at offset 328 partially underflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) Thread T150 (revalidator22) created by T0 here: #0 0x7f5b0f7f in __interceptor_pthread_create (/lib64/libasan.so.4+0x38f7f) #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792 #2 0xd23dc62f in udpif_start_threads ofproto/ofproto-dpif-upcall.c:639 #3 0xd23daf87 in ofproto_set_flow_table ofproto/ofproto-dpif-upcall.c:446 #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134 #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148 #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944 #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240 #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf) #9 0xd230a3d3 (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3) SUMMARY: AddressSanitizer: stack-buffer-overflow (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around the buggy address: 0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00 0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user:f7 Container overflow: fc Array cookie:ac Intra object redzone:bb ASan internal: fe Left alloca redzone: ca Right alloca redzone:cb ==57189==ABORTING --- ofproto/ofproto-dpif-upcall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index dc30824771..c2fc527a31 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1796,7 +1796,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif, } reval_seq = seq_read(udpif->reval_seq) - 1; /* Ensure revalidation. */ -ofpbuf_use_const(, >actions, flow->actions_len); +ofpbuf_use_const(, flow->actions, flow->actions_len); *ukey = ukey_create__(flow->key, flow->key_len,