New issue
Advanced search Search tips

Issue 808686 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Fix use after scope error in update_engine

Project Member Reported by manojgupta@chromium.org, Feb 2 2018

Issue description

My local asan build hit this:

update_engine-0.0.3-r3123:  * ASAN error detected:
update_engine-0.0.3-r3123:  * =================================================================
update_engine-0.0.3-r3123:  * ==54==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffe5f3bf081 at pc 0x5567feb9cb60 bp 0x7ffe5f3bf010 sp 0x7ffe5f3be7b0
update_engine-0.0.3-r3123:  * READ of size 7 at 0x7ffe5f3bf081 thread T0
update_engine-0.0.3-r3123:  *     #0 0x5567feb9cb5f in StrtolFixAndCheck(void*, char const*, char**, char*, int) ??:0:0
update_engine-0.0.3-r3123:  *     #1 0x5567febe8faf in strtol ??:0:0
update_engine-0.0.3-r3123:  *     #2 0x5567fec3aac2 in atol /build/amd64-generic/var/cache/portage/chromeos-base/update_engine/out/Default/../../../../../../../usr/include/stdlib.h:285:10
update_engine-0.0.3-r3123:  *     #3 0x5567fec3aac2 in chromeos_update_engine::UrlTerms::GetSizeT(long) const /build/amd64-generic/var/cache/portage/chromeos-base/update_engine/out/Default/../../../../../../../tmp/portage/chromeos-base/update_engine-0.0.3-r3123/work/update_engine-0.0.3/aosp/system/update_engine/test_http_server.cc:507:0
update_engine-0.0.3-r3123:  *     #4 0x5567fec38e4e in chromeos_update_engine::HandleConnection(int) /build/amd64-generic/var/cache/portage/chromeos-base/update_engine/out/Default/../../../../../../../tmp/portage/chromeos-base/update_engine-0.0.3-r3123/work/update_engine-0.0.3/aosp/system/update_engine/test_http_server.cc:525:34
update_engine-0.0.3-r3123:  *     #5 0x5567fec3b9a7 in main /build/amd64-generic/var/cache/portage/chromeos-base/update_engine/out/Default/../../../../../../../tmp/portage/chromeos-base/update_engine-0.0.3-r3123/work/update_engine-0.0.3/aosp/system/update_engine/test_http_server.cc:644:5
update_engine-0.0.3-r3123:  *     #6 0x7f1006711735 in __libc_start_main /var/tmp/portage/cross-x86_64-cros-linux-gnu/glibc-2.23-r15/work/glibc-2.23/csu/../csu/libc-start.c:289:0
update_engine-0.0.3-r3123:  *     #7 0x5567feb5c6a8 in _start ??:0:0
update_engine-0.0.3-r3123:  * 
update_engine-0.0.3-r3123:  * Address 0x7ffe5f3bf081 is located in stack of thread T0 at offset 33 in frame
update_engine-0.0.3-r3123:  *     #0 0x5567fec3a96f in chromeos_update_engine::UrlTerms::GetSizeT(long) const /build/amd64-generic/var/cache/portage/chromeos-base/update_engine/out/Default/../../../../../../../tmp/portage/chromeos-base/update_engine-0.0.3-r3123/work/update_engine-0.0.3/aosp/system/update_engine/test_http_server.cc:506:0
update_engine-0.0.3-r3123:  * 
update_engine-0.0.3-r3123:  *   This frame has 1 object(s):
update_engine-0.0.3-r3123:  *     [32, 56) 'ref.tmp.i' (line 501) <== Memory access at offset 33 is inside this variable
update_engine-0.0.3-r3123:  * HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
update_engine-0.0.3-r3123:  *       (longjmp and C++ exceptions *are* supported)
update_engine-0.0.3-r3123:  * SUMMARY: AddressSanitizer: stack-use-after-scope (/var/cache/portage/chromeos-base/update_engine/out/Default/test_http_server+0x69b5f)
update_engine-0.0.3-r3123:  * Shadow bytes around the buggy address:
update_engine-0.0.3-r3123:  *   0x10004be6fdc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fdd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fde0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fdf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fe00: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
update_engine-0.0.3-r3123:  * =>0x10004be6fe10:[f8]f8 f8 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fe20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
update_engine-0.0.3-r3123:  *   0x10004be6fe30: 00 00 00 00 00 00 00 00 f2 f2 f2 f2 f8 f8 f8 f8
update_engine-0.0.3-r3123:  *   0x10004be6fe40: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
update_engine-0.0.3-r3123:  *   0x10004be6fe50: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
update_engine-0.0.3-r3123:  *   0x10004be6fe60: f8 f2 f2 f2 f2 f2 f2 f2 f2 f2 00 00 f2 f2 00 00
update_engine-0.0.3-r3123:  * Shadow byte legend (one shadow byte represents 8 application bytes):
update_engine-0.0.3-r3123:  *   Addressable:           00
update_engine-0.0.3-r3123:  *   Partially addressable: 01 02 03 04 05 06 07
update_engine-0.0.3-r3123:  *   Heap left redzone:       fa
update_engine-0.0.3-r3123:  *   Freed heap region:       fd
update_engine-0.0.3-r3123:  *   Stack left redzone:      f1
update_engine-0.0.3-r3123:  *   Stack mid redzone:       f2
update_engine-0.0.3-r3123:  *   Stack right redzone:     f3
update_engine-0.0.3-r3123:  *   Stack after return:      f5
update_engine-0.0.3-r3123:  *   Stack use after scope:   f8
update_engine-0.0.3-r3123:  *   Global redzone:          f9
update_engine-0.0.3-r3123:  *   Global init order:       f6
update_engine-0.0.3-r3123:  *   Poisoned by user:        f7
update_engine-0.0.3-r3123:  *   Container overflow:      fc
update_engine-0.0.3-r3123:  *   Array cookie:            ac
update_engine-0.0.3-r3123:  *   Intra object redzone:    bb
update_engine-0.0.3-r3123:  *   ASan internal:           fe
update_engine-0.0.3-r3123:  *   Left alloca redzone:     ca
update_engine-0.0.3-r3123:  *   Right alloca redzone:    cb
update_engine-0.0.3-r3123:  * ==54==ABORTING

 
Identified the problem. Will send for review soon.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/6d2c71e261e331def8d41caea5a254f6312ece76

commit 6d2c71e261e331def8d41caea5a254f6312ece76
Author: Manoj Gupta <manojgupta@google.com>
Date: Sat Feb 03 12:44:09 2018

update_engine: Fix a stack use after scope issue.

GetCStr() returns the contents to a local string returned by Get().
The string is destroyed after the call to GetCStr() so the pointer
passed to atoi() is not valid.
Change Get() to return a reference instead so that it will stay valid
for lifetime of atoi().

BUG= chromium:808686 
TEST=No more asan complains.

Change-Id: Icc62064f4f3382a29ccbd36ca90f9952cf149364
Reviewed-on: https://chromium-review.googlesource.com/899512
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

[modify] https://crrev.com/6d2c71e261e331def8d41caea5a254f6312ece76/test_http_server.cc

Status: Verified (was: Untriaged)

Sign in to add a comment