Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Abort "attempting to call malloc_usable_size() for pointer which is not owned" with posix_memalign and malloc_usable_size #193

Closed
ramosian-glider opened this issue Aug 31, 2015 · 18 comments
Closed

Abort "attempting to call malloc_usable_size() for pointer which is not owned" with posix_memalign and malloc_usable_size #193

ramosian-glider opened this issue Aug 31, 2015 · 18 comments
Labels
Priority-Medium ProjectAddressSanitizer Status-Fixed Type-Defect

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 193

What steps will reproduce the problem?

1. Compile the following program with ASan:

#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>

int main() {
        void* ptr;
        if (!posix_memalign(&ptr, 16, 36)) {
                return malloc_usable_size(ptr);
        }
        return 0;
}


2. Run and observe error.

What is the expected output? What do you see instead?

Expected behavior is to not crash, however, we get this:

==32439==ERROR: AddressSanitizer: attempting to call malloc_usable_size() for pointer
which is not owned: 0x60500000efc0

What version of the product are you using? On what operating system?

Tested on LLVM r183459 and LLVM 3.3 branch, both on Ubuntu 12.04 LTS (64 bit).


Please provide any additional information below.

The issue does not reproduce for other values of alignment and size to posix_memalign.
I got these values from one of our failing Firefox tests. If you could fix this, then
we would highly appreciate if a fix could be backported to the 3.3 branch because we
plan to update to this branch soon. Thanks! :)

Reported by decoder.oh on 2013-06-07 00:07:42
@ramosian-glider
Copy link
Member Author 

It's not clear to me that calling malloc_usable_size on a posix_memalign'ed pointer
is safe in general.  I'm pretty sure it's safe with our allocator, however.

The right thing might be to make this behavior configurable.

Reported by justin.lebar on 2013-06-07 00:34:06

@ramosian-glider
Copy link
Member Author 

I see no reason why ASan should report an error for malloc_usable_size() on posix_memalign().
Looks like it's just a bug.

Reported by samsonov@google.com on 2013-06-07 10:56:54

  • Status changed: Accepted

@ramosian-glider
Copy link
Member Author 

The problem is:
when we're looking for allocation metadata (aka AsanChunk header), we don't know the
redzone size in advance, so we're trying to calculate redzone size from the actual
number of bytes we allocated. This doesn't work, as we calculate redzone size before
respecting alignment.

Quick fix for this may be: calculate redzone size after we increase the user-requested
size because of alignment. However, this may blow up the redzone sizes for allocations
with huge alignments.

Reported by samsonov@google.com on 2013-06-07 13:33:42

@ramosian-glider
Copy link
Member Author 

I am on it, will take closer look on Monday. Thanks for the report.

Reported by konstantin.s.serebryany on 2013-06-07 13:35:49

  • Status changed: Started

@ramosian-glider
Copy link
Member Author 

BTW, you may use ASAN_OPTIONS=check_malloc_usable_size=0 as a temporary workaround.

Reported by konstantin.s.serebryany on 2013-06-07 15:37:35

@ramosian-glider
Copy link
Member Author 

Should be fixed by http://llvm.org/viewvc/llvm-project?rev=183647&view=rev

Reported by konstantin.s.serebryany on 2013-06-10 10:49:34

  • Status changed: Fixed

@ramosian-glider
Copy link
Member Author 

Fix confirmed, thanks. We will try and see if we can backport this to the 3.3 branch.

Reported by decoder.oh on 2013-06-10 12:48:24

@ramosian-glider
Copy link
Member Author 

kcc, we have tried to port this to the 3.3 branch but we get a startup crash with our
patch. Would you mind helping us to port this to 3.3? It could also be landed in a
maintenance branch then once it has been opened.

Not sure what is easier for you, taking a look at our patch or just creating the backport
yourself.

Reported by decoder.oh on 2013-06-19 16:30:46

@ramosian-glider
Copy link
Member Author 

Please let us see your start-up crash (stack trace)

Also, Alexey just added another small fix in the same general area.
http://llvm.org/viewvc/llvm-project?rev=184404&view=rev
W/o this change we had a crash in LSan, but theoretically it could cause a crash in
ASan too.

Reported by konstantin.s.serebryany on 2013-06-20 09:01:58

@ramosian-glider
Copy link
Member Author 

I figured that the trace wouldn't be very helpful to you. It's related to memalign:

    ==2677==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000033450
at pc 0x7fa1925865ea bp 0x7fffc3c349c0 sp 0x7fffc3c349b8
    READ of size 4 at 0x602000033450 thread T0
        #0 0x7fa1925865e9 in NS_IsMainThread() /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/xpcom/ds/../../dist/include/nsThreadUtils.h:104
        #1 0x7fa18ee71923 in nsPrefBranch /builds/slave/try-l64-0000000000000000000000/build/modules/libpref/src/nsPrefBranch.cpp:82
        #2 0x7fa18ee6126b in mozilla::Preferences::GetInstanceForService() /builds/slave/try-l64-0000000000000000000000/build/modules/libpref/src/Preferences.cpp:237
        #3 0x7fa18ee7d908 in PreferencesConstructor(nsISupports*, nsID const&, void**)
/builds/slave/try-l64-0000000000000000000000/build/modules/libpref/src/nsPrefsFactory.cpp:13
        #4 0x7fa1925fe5db in nsComponentManagerImpl::CreateInstanceByContractID(char
const*, nsISupports*, nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:10
    93
        #5 0x7fa1925f59e9 in nsComponentManagerImpl::GetServiceByContractID(char const*,
nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:1449
        #6 0x7fa19254718c in CallGetService(char const*, nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/xpcom/build/nsComponentManagerUtils.cpp:60
        #7 0x7fa192542d8b in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID,
nsID const&) /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/xpcom/build/nsCOMPtr.cpp:92
        #8 0x7fa19256a080 in nsCOMPtr /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/chrome/src/../../dist/include/nsCOMPtr.h:640
        #9 0x7fa19256912b in nsChromeRegistry::GetSingleton() /builds/slave/try-l64-0000000000000000000000/build/chrome/src/nsChromeRegistry.cpp:645
        #10 0x7fa192560408 in nsChromeRegistryConstructor(nsISupports*, nsID const&,
void**) /builds/slave/try-l64-0000000000000000000000/build/xpcom/build/nsXPComInit.cpp:252
        #11 0x7fa1925fe5db in nsComponentManagerImpl::CreateInstanceByContractID(char
const*, nsISupports*, nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:1093
        #12 0x7fa1925f59e9 in nsComponentManagerImpl::GetServiceByContractID(char const*,
nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:1449
        #13 0x7fa19254718c in CallGetService(char const*, nsID const&, void**) /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/xpcom/build/nsComponentManagerUtils.cpp:60
        #14 0x7fa192542d8b in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID,
nsID const&) /builds/slave/try-l64-0000000000000000000000/build/obj-firefox/xpcom/build/nsCOMPtr.cpp:92
        #15 0x7fa192558bfe in nsCOMPtr /builds/slave/try-l64-0000000000000000000000/build/xpcom/build/../glue/nsCOMPtr.h:640
        #16 0x7fa1925eca73 in ParseManifest(NSLocationType, mozilla::FileLocation&,
char*, bool) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/ManifestParser.cpp:626
        #17 0x7fa1925fa185 in nsComponentManagerImpl::RegisterManifest(NSLocationType,
mozilla::FileLocation&, bool) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:556
        #18 0x7fa1925fa44f in nsComponentManagerImpl::ManifestManifest(nsComponentManagerImpl::ManifestProcessingContext&,
int, char* const*) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:569
        #19 0x7fa1925eccbf in ParseManifest(NSLocationType, mozilla::FileLocation&,
char*, bool) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/ManifestParser.cpp:645
        #20 0x7fa1925fa185 in nsComponentManagerImpl::RegisterManifest(NSLocationType,
mozilla::FileLocation&, bool) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:556
        #21 0x7fa1925f800e in nsComponentManagerImpl::RereadChromeManifests(bool) /builds/slave/try-l64-0000000000000000000000/build/xpcom/components/nsComponentManager.cpp:732
        #22 0x7fa19255c4fb in NS_InitXPCOM2 /builds/slave/try-l64-0000000000000000000000/build/xpcom/build/nsXPComInit.cpp:467
        #23 0x7fa18e6c9c42 in ScopedXPCOMStartup::Initialize() /builds/slave/try-l64-0000000000000000000000/build/toolkit/xre/nsAppRunner.cpp:1189
        #24 0x7fa18e6cabc5 in XRE_main /builds/slave/try-l64-0000000000000000000000/build/toolkit/xre/nsAppRunner.cpp:4126
        #25 0x43177e in do_main(int, char**, nsIFile*) /builds/slave/try-l64-0000000000000000000000/build/browser/app/nsBrowserApp.cpp:272
        #26 0x7fa19a0f976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
        #27 0x4309ec in _start ??:0
    0x602000033450 is located 0 bytes inside of 4-byte region [0x602000033450,0x602000033454)
    allocated by thread T0 here:
        #0 0x422ae9 in memalign ??:0
        #1 0x7fa19b2d1364 in allocate_and_init /build/buildd/eglibc-2.15/elf/dl-tls.c:526
    Shadow bytes around the buggy address:
      0x0c047fffe630: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fffe640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fffe650: fa fa 00 01 fa fa 00 06 fa fa fd fd fa fa 00 fa
      0x0c047fffe660: fa fa fd fd fa fa 00 fa fa fa 00 02 fa fa 00 01
      0x0c047fffe670: fa fa 05 fa fa fa 00 fa fa fa fd fd fa fa 00 fa
    =>0x0c047fffe680: fa fa 00 04 fa fa 00 04 fa fa[04]fa fa fa 00 fa
      0x0c047fffe690: fa fa 00 fa fa fa 00 00 fa fa 00 00 fa fa 00 00
      0x0c047fffe6a0: fa fa 00 00 fa fa 00 00 fa fa 00 fa fa fa fd fa
      0x0c047fffe6b0: fa fa fd fd fa fa fd fd fa fa 00 00 fa fa 00 00
      0x0c047fffe6c0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
      0x0c047fffe6d0: fa fa 00 00 fa fa fd fd fa fa fd fd fa fa fd fd
    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
      Heap right redzone:    fb
      Freed heap region:     fd
      Stack left redzone:    f1
      Stack mid redzone:     f2
      Stack right redzone:   f3
      Stack partial redzone: f4
      Stack after return:    f5
      Stack use after scope: f8
      Global redzone:        f9
      Global init order:     f6
      Poisoned by user:      f7
      ASan internal:         fe
    ==2677==ABORTING

Reported by decoder.oh on 2013-06-20 09:28:03

@ramosian-glider
Copy link
Member Author 

extremely weird crash. Are you saying that you don't see this crash with 3.3 (w/o the
patch) and with trunk?

Reported by konstantin.s.serebryany on 2013-06-20 09:36:39

@ramosian-glider
Copy link
Member Author 

Yep, I tried with trunk yesterday and I did not see it. I also did not see this with
the last 3.3 branch I tried, but I will try again now with and without the patch to
exactly confirm it's just the patch.

Reported by decoder.oh on 2013-06-20 09:46:26

@ramosian-glider
Copy link
Member Author 

I tried reproducing locally now and didn't succeed so far :( maybe there's a libc difference
to the build machines. I'm still pretty sure though that it's the patch because the
last build (without the patch) worked fine and we didn't change anything else. Also
the memalign in the trace is suspicious.

would it help you to see the backported patch? Thanks for any help :)

Reported by decoder.oh on 2013-06-20 18:44:40

@ramosian-glider
Copy link
Member Author 

>> would it help you to see the backported patch?
sure

Reported by konstantin.s.serebryany on 2013-06-20 18:52:06

@ramosian-glider
Copy link
Member Author 

Okay :) We're tracking our Clang 3.3 update here:

https://bugzilla.mozilla.org/show_bug.cgi?id=870173

The backport patch is in the first attachment (it's a patch creating this and another
patch in our tree, you can probably just ignore the rest and only look at the actual
patch).

The second attachment is the manifest which contains the revision number for the latest
build used (including the patch).

Let me know if you need anything else :)

Reported by decoder.oh on 2013-06-21 07:12:00

@ramosian-glider
Copy link
Member Author 

The patch looks ok, it's hard to tell why it crashes when applied to 3.3
(while not crashing in trunk). 

Could you remind us why do you have to use 3.3 for asan?
For Chrome we always use recent trunk and have no such problems.

Reported by konstantin.s.serebryany on 2013-06-21 09:06:43

@ramosian-glider
Copy link
Member Author 

I'm hitting the NS_IsMainThread problem (comment 10) with LLVM trunk.  I filed http://code.google.com/p/address-sanitizer/issues/detail?id=204.

Reported by jruderman on 2013-06-29 14:35:56

@ramosian-glider
Copy link
Member Author 

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:13:41

  • Labels added: ProjectAddressSanitizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Priority-Medium ProjectAddressSanitizer Status-Fixed Type-Defect
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ramosian-glider