Tarantool development patches archive
 help / color / mirror / Atom feed
From: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH msgpack v1] test: correct buffer size to fix ASAN error
Date: Fri, 4 Sep 2020 14:07:36 +0300	[thread overview]
Message-ID: <20200904110736.GA13894@hpalx> (raw)
In-Reply-To: <4be7d009-a6b3-0bee-17b6-c225d64f520a@tarantool.org>

Hi Vlad, thanks for the review. I've made all your suggestions.

On Mon, Aug 24, 2020 at 11:49:55PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 2 comments below.
> 
> On 20.08.2020 10:02, Alexander V. Tikhonov wrote:
> > Found ASAN error:
> > 
> > [001] +    ok 206 - =================================================================
> > [001] +==6889==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x604000000031 at pc 0x0000005a72e7 bp 0x7ffe47c30c80 sp 0x7ffe47c30c78
> > [001] +WRITE of size 1 at 0x604000000031 thread T0
> > [001] +    #0 0x5a72e6 in mp_store_u8 /tarantool/src/lib/msgpuck/msgpuck.h:258:1
> > [001] +    #1 0x5a72e6 in mp_encode_uint /tarantool/src/lib/msgpuck/msgpuck.h:1768
> > [001] +    #2 0x4fa657 in test_mp_print /tarantool/src/lib/msgpuck/test/msgpuck.c:957:16
> > [001] +    #3 0x509024 in main /tarantool/src/lib/msgpuck/test/msgpuck.c:1331:2
> > [001] +    #4 0x7f3658fd909a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
> > [001] +    #5 0x41f339 in _start (/tnt/test/unit/msgpack.test+0x41f339)
> > [001] +
> > [001] +0x604000000031 is located 0 bytes to the right of 33-byte region [0x604000000010,0x604000000031)
> > [001] +allocated by thread T0 here:
> > [001] +    #0 0x4cace3 in malloc (/tnt/test/unit/msgpack.test+0x4cace3)
> > [001] +    #1 0x4fa5db in test_mp_print /tarantool/src/lib/msgpuck/test/msgpuck.c:945:18
> > [001] +    #2 0x509024 in main /tarantool/src/lib/msgpuck/test/msgpuck.c:1331:2
> > [001] +    #3 0x7f3658fd909a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
> > [001] +
> > [001] +SUMMARY: AddressSanitizer: heap-buffer-overflow /tarantool/src/lib/msgpuck/msgpuck.h:258:1 in mp_store_u8
> > [001] +Shadow bytes around the buggy address:
> > [001] +  0x0c087fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [001] +  0x0c087fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [001] +  0x0c087fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [001] +  0x0c087fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [001] +  0x0c087fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [001] +=>0x0c087fff8000: fa fa 00 00 00 00[01]fa fa fa fa fa fa fa fa fa
> > [001] +  0x0c087fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > [001] +  0x0c087fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > [001] +  0x0c087fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > [001] +  0x0c087fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > [001] +  0x0c087fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > [001] +Shadow byte legend (one shadow byte represents 8 application bytes):
> > [001] +  Addressable:           00
> > [001] +  Partially addressable: 01 02 03 04 05 06 07
> > [001] +  Heap left redzone:       fa
> > [001] +  Freed heap region:       fd
> > [001] +  Stack left redzone:      f1
> > [001] +  Stack mid redzone:       f2
> > [001] +  Stack right redzone:     f3
> > [001] +  Stack after return:      f5
> > [001] +  Stack use after scope:   f8
> > [001] +  Global redzone:          f9
> > [001] +  Global init order:       f6
> > [001] +  Poisoned by user:        f7
> > [001] +  Container overflow:      fc
> > [001] +  Array cookie:            ac
> > [001] +  Intra object redzone:    bb
> > [001] +  ASan internal:           fe
> > [001] +  Left alloca redzone:     ca
> > 
> > Invetigated the buffer size that was allocated - it was 33 bytes, but it
> 
> 1. Invetigated -> Investigated.
> 
> > needed 34. The fix was to increase this buffer.
> > 
> > Part of tarantool/tarantool#4360
> > ---
> > 
> > Github: https://github.com/tarantool/msgpuck/tree/avtikhon/gh-4360-fix-asan-error
> > Issue: https://github.com/tarantool/tarantool/issues/4360
> > 
> >  test/msgpuck.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/test/msgpuck.c b/test/msgpuck.c
> > index 2d9bcbf..6613234 100644
> > --- a/test/msgpuck.c
> > +++ b/test/msgpuck.c
> > @@ -940,7 +940,7 @@ test_mp_print()
> >  
> >  	/* Test mp_snprint max nesting depth. */
> >  	int mp_buff_sz = MP_PRINT_MAX_DEPTH * mp_sizeof_array(1) +
> > -			 mp_sizeof_uint(1);
> > +			 mp_sizeof_uint(1) + 1;
> 
> 2. Better change to this:
> 
> 	int mp_buff_sz = (MP_PRINT_MAX_DEPTH + 1) * mp_sizeof_array(1) +
> 			 mp_sizeof_uint(1);
> 
> To emphasize, that +1 comes from one another mp_encode_array(1).
> 
> >  	int exp_str_sz = 2 * (MP_PRINT_MAX_DEPTH + 1) + 3 + 1;
> >  	char *mp_buff = malloc(mp_buff_sz);
> >  	char *exp_str = malloc(exp_str_sz);

      reply	other threads:[~2020-09-04 11:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-20  8:02 Alexander V. Tikhonov
2020-08-24 21:49 ` Vladislav Shpilevoy
2020-09-04 11:07   ` Alexander V. Tikhonov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200904110736.GA13894@hpalx \
    --to=avtikhon@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH msgpack v1] test: correct buffer size to fix ASAN error' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox