Closed Bug 820316 Opened 12 years ago Closed 12 years ago

Out of range read in memcpy called from android::GraphicBuffer::flatten

Categories

(Core :: Graphics: Layers, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 867996

People

(Reporter: jseward, Assigned: mikeh)

References

Details

(4 keywords)

b2g running on the emulator, every startup. I don't know if this is a bug in our code (as in: is this a bug in the android libs, or did we allocate a buffer of the wrong size?, etc), but filing anyway. Treat the stacks with skepticism. They are generated by stack scanning (a nasty kludge at the best of times) and may mention functions which aren't really present in the stacks. Invalid read of size 4 at 0x480AFA0: memcpy (mc_replace_strmem.c:883) by 0x4D92ADD: android::GraphicBuffer::flatten(void*, unsigned int, int*, unsigned int) const (in /system/lib/libui.so) by 0x48C8A45: std::__node_alloc::_M_allocate(unsigned int&) (in /system/lib/libstlport.so) by 0x4806C0B: realloc (vg_replace_malloc.c:662) by 0x5C52CE7: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::insert_unique_noresize(std::pair<int const, IPC::Channel::Listener*> const&) (_hashtable.c:223) by 0x5C7F681: mozilla::layers::PLayersParent::Write(mozilla::layers::MaybeMagicGrallocBufferHandle const&, IPC::Message*) (ipc_message_utils.h:74) by 0x5DD15C3: IPC::Message::Message(int, unsigned int, IPC::Message::PriorityValue, IPC::Message::MessageCompression, char const*) (ipc_message.cc:32) by 0x5C83267: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (PLayersParent.cpp:467) by 0x5C82D57: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&) (nsTArray.h:735) by 0x5C416B3: mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (RPCChannel.cpp:400) Address 0x8669344 is 8 bytes after a block of size 76 alloc'd at 0x48071F4: operator new[](unsigned int) (vg_replace_malloc.c:357) by 0x6BF828B: ??? (in /system/lib/hw/gralloc.goldfish.so) by 0x4D92C31: android::GraphicBuffer::initSize(unsigned int, unsigned int, int, unsigned int) (in /system/lib/libui.so) by 0x4D92FA5: android::GraphicBuffer::GraphicBuffer(unsigned int, unsigned int, int, unsigned int) (in /system/lib/libui.so) by 0x5E450E9: mozilla::layers::GrallocBufferActor::Create(nsIntSize const&, gfxASurface::gfxContentType const&, mozilla::layers::MaybeMagicGrallocBufferHandle*) (ShadowLayerUtilsGralloc.cpp:206) by 0x5C52BF1: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::_M_insert_noresize(unsigned int, std::pair<int const, IPC::Channel::Listener*> const&) (_alloc.h:158) by 0x5E43747: mozilla::layers::ShadowLayersParent::AllocPGrallocBuffer(nsIntSize const&, gfxASurface::gfxContentType const&, mozilla::layers::MaybeMagicGrallocBufferHandle*) (ShadowLayersParent.cpp:502) by 0x5C52CE7: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::insert_unique_noresize(std::pair<int const, IPC::Channel::Listener*> const&) (_hashtable.c:223) by 0x5C82D57: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&) (nsTArray.h:735) by 0x5C416B3: mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (RPCChannel.cpp:400)
The above shows a 32-bit read 8 bytes after the end of the block. In the same run I also see reads with the same stack at 4 past the end of the block and 0 past the end of the block.
Here's my best guess at cleaned-up stacks. Invalid read of size 4 at 0x480AFA0: memcpy (mc_replace_strmem.c:883) by 0x4D92ADD: android::GraphicBuffer::flatten(void*, unsigned int, int*, unsigned int) const (in /system/lib/libui.so) by 0x5C52CE7: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::insert_unique_noresize(std::pair<in const, IPC::Channel::Listener*> const&) (_hashtable.c:223) by 0x5C7F681: mozilla::layers::PLayersParent::Write( mozilla::layers::MaybeMagicGrallocBufferHandle const&, IPC::Message*) (ipc_message_utils.h:74) by 0x5DD15C3: IPC::Message::Message(int, unsigned int, IPC::Message::PriorityValue, IPC::Message::MessageCompression, char const*) (ipc_message.cc:32 by 0x5C83267: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (PLayersParent.cpp:467) by 0x5C82D57: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&) (nsTArray.h:735) by 0x5C416B3: mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (RPCChannel.cpp:400) Address 0x8669344 is 8 bytes after a block of size 76 alloc'd at 0x48071F4: operator new[](unsigned int) (vg_replace_malloc.c:357) by 0x6BF828B: ??? (in /system/lib/hw/gralloc.goldfish.so) by 0x4D92C31: android::GraphicBuffer::initSize(unsigned int, unsigned int, int, unsigned int) (in /system/lib/libui.so) by 0x4D92FA5: android::GraphicBuffer::GraphicBuffer(unsigned int, unsigned int, int, unsigned int) (in /system/lib/libui.so) by 0x5E450E9: mozilla::layers::GrallocBufferActor::Create(nsIntSize const&, gfxASurface::gfxContentType const&, mozilla::layers::MaybeMagicGrallocBufferHandle*) (ShadowLayerUtilsGralloc.cpp:206) by 0x5C52BF1: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::_M_insert_noresize(unsigned int, std::pair<int const, IPC::Channel::Listener*> const&) (_alloc.h:158) by 0x5E43747: mozilla::layers::ShadowLayersParent::AllocPGrallocBuffer(nsIntSize const&, gfxASurface::gfxContentType const&, mozilla::layers::MaybeMagicGrallocBufferHandle*) (ShadowLayersParent.cpp:502) by 0x5C52CE7: std::hashtable<std::pair<int const, IPC::Channel::Listener*>, int, std::hash<int>, std::priv::_HashMapTraitsT<std::pair<int const, IPC::Channel::Listener*> >, std::priv::_Select1st<std::pair<int const, IPC::Channel::Listener*> >, std::equal_to<int>, std::allocator<std::pair<int const, IPC::Channel::Listener*> > >::insert_unique_noresize(std::pair<int const, IPC::Channel::Listener*> const&) (_hashtable.c:223) by 0x5C82D57: mozilla::layers::PLayersParent::OnMessageReceived(IPC::Message const&) (nsTArray.h:735) by 0x5C416B3: mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (RPCChannel.cpp:400)
According to hg blame it likely comes from bug 765734 changeset: http://hg.mozilla.org/mozilla-central/rev/089b9510e595
Depends on: 765734
Chris want to take this one?
Flags: needinfo?(jones.chris.g)
picking chris based on likely regressor.
Assignee: nobody → jones.chris.g
Flags: needinfo?(jones.chris.g)
Let's try to keep Chris out of this :-)
Assignee: jones.chris.g → kchen
We just went over "six weeks" with this one - does it still happen? Kan-Ru, is this something you can look at in the short term? Let us know either way.
I've never saw this on real devices. I would like to know if this is still reproducible on the emulator.
(In reply to Kan-Ru Chen [:kanru] from comment #8) > I've never saw this on real devices. I would like to know if this is still > reproducible on the emulator. Kan-Ru, I just added qawanted, let me know if you're asking somebody else to reproduce instead.
Keywords: qawanted
ccing Tony to help find QA for this.
Yes. We need to know if this is still reproducible in our emulator and/or devices. It looks like it's a issue for a specific revision of the android library.
I've been investigating a crash in ::flatten() in bug 867996 (and by extension, bug 818103) that _only_ happens on automated-test builds (e.g. the try server) but not on local emulator builds. If these two issues are related (and it looks strongly like they are), this is still happening: see https://bugzilla.mozilla.org/show_bug.cgi?id=818103#c686
In ::flatten()[1], the memcpy() is copying data out of an internal structure and into user-supplied buffers, the latter of which are sized based on information obtained by calling GraphicBuffer::getFlattenedSize()[2] and ::setFdCount()[3]. 1. https://github.com/android/platform_frameworks_base/blob/ics-mr1-release/libs/ui/GraphicBuffer.cpp#L208 2. https://github.com/android/platform_frameworks_base/blob/ics-mr1-release/libs/ui/GraphicBuffer.cpp#L200 3. https://github.com/android/platform_frameworks_base/blob/ics-mr1-release/libs/ui/GraphicBuffer.cpp#L204 The source buffer object is type 'native_handle_t'[4]. 4. https://github.com/marcofreda527/jb412gecko/blob/8b11d14dfa0dc5876eba1b5ca8a76ff88e10c839/media/omx-plugin/include/ics/cutils/native_handle.h#L30 Does Valgrind say where these objects are being allocated?
Looks like the native_handle_t object is being allocated by a call to mallocDev->alloc(), a reference to which is obtained by calling hw_get_module(GRALLOC_HARDWARE_MODULE_ID) and gralloc_open(). jseward, how did you obtain the output in comment 0? Valgrind running in the emulator?
Flags: needinfo?(jseward)
Okay, I have Valgrind running on a local emulator build (*PHEW!*) and with some extra debug prints in ParamTraits<MagicGrallocBufferHandle>::Write() and GrallocBufferActor::Create(), I see: I( 100:0x83) aSize.width=320 .height=480 format=2 E( 100:0x83) mikeh: alloc: w=320 h=480 E( 100:0x83) mikeh: handle=0xc2e6c90, stride=320 I( 100:0x83) buffer.get()=0xc519000 I( 100:0x83) Trying to get flattenable, aMsg=0xaa7f8a0 I( 100:0x83) flattenable=0xc519000 I( 100:0x83) nbytes=104 I( 100:0x83) nfds=1 I( 100:0x83) flattening E( 100:0x83) mikeh: h=0xc2e6c90 ->numFds=1 ->numInts=18 ->data=0xc2e6c9c ->data+numFds=0xc2e6ca0 I( 100:0x83) write nbytes I( 100:0x83) write size I( 100:0x83) write data I( 100:0x83) write fd 0 This is on start-up, and Valgrind reports: ==100== Invalid read of size 4 ==100== at 0x480AF1C: memcpy (mc_replace_strmem.c:883) ==100== by 0x4CE7B03: android::GraphicBuffer::flatten(void*, unsigned int, int*, unsigned int) const (GraphicBuffer.cpp:240) ==100== by 0x630B8B1: IPC::ParamTraits<mozilla::layers::MagicGrallocBufferHandle>::Write(IPC::Message*, mozilla::layers::MagicGrallocBufferHandle const&) (ShadowLayerUtilsGralloc.cpp:56) ==100== by 0x60580FD: mozilla::layers::PLayerTransactionParent::Write(mozilla::layers::MaybeMagicGrallocBufferHandle const&, IPC::Message*) (ipc_message_utils.h:115) ==100== by 0x605B2FB: mozilla::layers::PLayerTransactionParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (PLayerTransactionParent.cpp:551) ==100== by 0x605301B: mozilla::layers::PCompositorParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) (PCompositorParent.cpp:361) ==100== by 0x6015CC3: mozilla::ipc::SyncChannel::OnDispatchMessage(IPC::Message const&) (SyncChannel.cpp:145) ==100== by 0x601466F: mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (RPCChannel.cpp:400) ==100== by 0x5FF31FB: RunnableMethod<mozilla::dom::TabChild, void (mozilla::dom::TabChild::*)(), Tuple0>::Run() (tuple.h:383) ==100== by 0x6012EC9: mozilla::ipc::RPCChannel::DequeueTask::Run() (RPCChannel.h:425) ==100== by 0x629710D: MessageLoop::RunTask(Task*) (message_loop.cc:337) ==100== by 0x62980DF: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (message_loop.cc:345) ==100== Address 0xc2e6ce0 is 0 bytes after a recently re-allocated block of size 80 alloc'd ==100== at 0x4815B18: arena_malloc (jemalloc.c:4169) ==100== by 0x4815CCF: imalloc (jemalloc.c:4247) ==100== by 0x4815CF3: je_malloc (jemalloc.c:6302) ==100== by 0x48120D3: malloc (replace_malloc.c:150) ==100== by 0x4811D8F: operator new[](unsigned int) (mozmemory_wrap.c:20) ==100== by 0x77C428B: ??? (in /system/lib/hw/gralloc.goldfish.so) The address 0xc2e6ce0 is exactly 80 bytes after the native_handle_t at 0xc2e6c90, and GraphicBuffer.cpp:240 is: memcpy(&buf[8], h->data + h->numFds, h->numInts*sizeof(int)); If all of this is accurate, it means the native_handle_t object (which includes a terminal 'int data[0]' member) that is being allocated by gralloc.goldfish.so is either too small, or has its 'numInts' member too high by 1.
It looks like sizeof( cb_handle_t ) = 76; cb_handle_t is a subclass of native_handle[_t], which has sizeof(native_handle_t)=12. This means numInts _should_ be 64/6 = 16, not the 18 we are actually getting*. *based on nbytes=104 as seen in comment 15[1]. 1. http://androidxref.com/4.0.4/xref/frameworks/base/libs/ui/GraphicBuffer.cpp#200
(Ahem... 64/4 = 16.)
Okay, here's what's going on: when the cb_handle_t object is created, it starts off with numFds=0 and numInts=sizeof(cb_handle_t)/sizeof(int)=76/4=19. Eventually gralloc_alloc() calls cb_handle_t.setFd(), which sets numFds=1 and numInts=(76-4)/4=18. When this object makes it to ::flatten(), that function calls: memcpy(&buf[8], h->data + h->numFds, h->numInts*sizeof(int)); h->data points to the beginning of the ints that are specific to cb_handle_t. These appear in memory after the members of native_handle_t, but h->numInts _includes_ the size of the native_handle_t superclass members as well, so this memcpy() will _always_ read off the end of the object. This is a pretty horrid bug, but all of it exists in AOSP code, so there's no simple fix for us.
A fix for the emulator landed in bug 867996: https://github.com/mozilla-b2g/android-development/pull/2 Bug 818103 will remain open to track uploading a new emulator build to the infrastructure. With the above patch, I get a clean Valgrind run (at least as far as ::flatten() is concerned), I'm going to close this as a duplicate.
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: needinfo?(jseward)
Resolution: --- → DUPLICATE
Assignee: kchen → mhabicher
Group: core-security
You need to log in before you can comment on or make changes to this bug.