Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

2019-12-02 Thread Linhaifeng
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

2019-12-02 Thread Ben Pfaff
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

2019-11-29 Thread Numan Siddique
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

2019-11-28 Thread Linhaifeng
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

2019-11-28 Thread 0-day Robot
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

2019-11-28 Thread Linhaifeng
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,