867996 - Crash in GraphicBuffer::flatten()

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Mike Habicher [:mikeh] (high bugzilla latency)]

I'm filing this bug so that any discussions/findings/comments/reviews will be free from >600 TBPL notifications.

What we know so far (thanks, :marshall_law):
- crash only happens on TBPL, so far unable to reproduce locally
- with this patch[1] applied,
- we see this output[2] (scroll to the bottom for logcat output)

1. https://hg.mozilla.org/try/rev/16abce400930
2. https://tbpl.mozilla.org/php/getParsedLog.php?id=21950180&tree=Try&full=1#error1

- crash happens when we call |flattenable->flatten(data, nbytes, fds, nfds)| [3], which is part of the AOSP layer
- TBPL uses a custom toolchain that we can't easily modify (see [4])

3. https://hg.mozilla.org/try/rev/16abce400930#l1.41
4. https://bugzilla.mozilla.org/show_bug.cgi?id=818103#c416

Comment 1

[Dave Hylands [:dhylands]]

I noticed that the crash is occuring at address 0x44400000

It would be useful to know if this is supposed to be beginning of the buffer (which would indicate that it was probably already freed), or if the buffer is earlier in memory and we're going past the end (and triggering the segfault on the first byte of a non-mapped page).

Comment 2

[Mike Habicher [:mikeh] (high bugzilla latency)]

dhylands, that's easy enough to test.

Another data point: apparently this doesn't happen on the b2g18 branch, only on mozilla-central.

Comment 3

[Andrew Halberstadt [:ahal]]

Another data point, we started seeing this sometime around New Years +/- a week. We didn't have crash detection back then, but I think it was the same problem. All I know for sure is that this didn't use to happen, and then it did.

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

dhylands, the buffer that the header+data is written into is allocated on the stack:
https://hg.mozilla.org/try/rev/16abce400930#l1.37

I've looked through this code a dozen times, and it seems to me that unless one of the GraphicBuffer members is modified asynchronously, this crash can't happen.

Comment 5

[Kan-Ru Chen [:kanru] (UTC+9)]

Bug 820316 might be related.

Comment 6

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Kan-Ru Chen [:kanru] from comment #5)
>
> Bug 820316 might be related.

I can't see that bug. CC me?

Comment 7

[Mike Habicher [:mikeh] (high bugzilla latency)]

Exercising a potential emulator fix:
https://tbpl.mozilla.org/?tree=Try&rev=a229b5394bb1

Comment 8

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Emulator patch

Drop this patch into $B2G/development and:

patch -p1 < 867996_2013-05-13.patch

Comment 9

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 748961 [details] [diff] [review]
Emulator patch

Never mind--git grabbed the wrong file. Stand by.

Comment 10

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Emulator patch (v2)

Drop this patch into $B2G/development and:

patch -p1 < 867996_2013-05-13.patch

Comment 11

[Mike Habicher [:mikeh] (high bugzilla latency)]

https://tbpl.mozilla.org/?tree=Try&rev=d1fdb967f177

Comment 12

[Jonathan Griffin (:jgriffin)]

Comment on attachment 748964 [details] [diff] [review]
Emulator patch (v2)

Review of attachment 748964 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing f? since I've made the emulator with this patch.

Comment 13

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Link to PR to fix emulator out-of-bounds read

Migrate the fix to this bug, since bug 818103's history is pretty messy.

Comment 15

[Mike Habicher [:mikeh] (high bugzilla latency)]

Bug 818103 will remain open to track uploading a new emulator build to the tinderbox.