Closed Bug 1039166 Opened 5 years ago Closed 5 years ago

Intermittent | gtest | test failed with return code 1 | SUMMARY: AddressSanitizer: use-after-poison ??:0 ?? during pkixocsp_VerifyEncodedOCSPResponse

Categories

(Core :: Security: PSM, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox32 --- unaffected
firefox33 --- fixed
firefox34 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected

People

(Reporter: cbook, Assigned: briansmith)

References

()

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Linux x86-64 mozilla-inbound asan build on 2014-07-15 18:02:21 PDT for push bc0d9b08d24e

slave: bld-linux64-spot-1041

https://tbpl.mozilla.org/php/getParsedLog.php?id=43873592&tree=Mozilla-Inbound

not really sure if this should be a s-s bug ?

SUMMARY: AddressSanitizer: use-after-poison ??:0 ??
gtest TEST-UNEXPECTED-FAIL | gtest | test failed with return code 1


SUMMARY: AddressSanitizer: use-after-poison ??:0 ??
Shadow bytes around the buggy address:
  0x0c3a7fff8910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a7fff8950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3a7fff8960: 00 00 00 00 00 00 00[07]f7 f7 f7 f7 f7 f7 f7 f7
  0x0c3a7fff8970: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x0c3a7fff8980: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x0c3a7fff8990: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x0c3a7fff89a0: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x0c3a7fff89b0: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
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
  Contiguous container OOB:fc
  ASan internal:           fe
Can't help but notice that this is happening in the PSM tests.
Component: General → Security: PSM
Flags: needinfo?(dkeeler)
Flags: needinfo?(cviecco)
Flags: needinfo?(brian)
Conveniently, a test added yesterday in bug 916629.
Blocks: 916629
Flags: needinfo?(cviecco)
Depends on: 1039633
Summary: Intermittent | gtest | test failed with return code 1 | SUMMARY: AddressSanitizer: use-after-poison ??:0 ?? → Intermittent | gtest | test failed with return code 1 | SUMMARY: AddressSanitizer: use-after-poison ??:0 ?? during pkixocsp_VerifyEncodedOCSPResponse
Off the top of my head, I don't know what's wrong. I'll have more time to look into this tomorrow if Brian doesn't get to it first.
Flags: needinfo?(dkeeler)
This is almost definitely a problem in the testing code, not a problem in the code being tested. I will look into it and fix it in the next few days. If the intermittent failures become too common we can disable this test, but it doesn't look too bad yet.
Assignee: nobody → brian
Flags: needinfo?(brian)
Target Milestone: --- → mozilla33
Any updates here, Brian? :)
Flags: needinfo?(brian)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #81)
> Any updates here, Brian? :)

I am out of the office until next week. I will fix the bug then.
Flags: needinfo?(brian)
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #82)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #81)
> > Any updates here, Brian? :)
> 
> I am out of the office until next week. I will fix the bug then.

Update: I have some patches queued up which are likely to indirectly fix this. Should be ready in a few days.
Not sure what to make of this now that we're getting stacks...
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #107)
> Update: I have some patches queued up which are likely to indirectly fix
> this. Should be ready in a few days.

Any news here? We're getting close to the next uplift now.
Target Milestone: mozilla33 → ---
We have an uplift next week. We were promised a fix nearly a month ago. I'm not willing to take these failures onto another release. I think we've been exceedingly patient waiting on a fix here. If I don't hear anything here *very* soon, I'll be backing out bug 916629 from m-c and Aurora.
Flags: needinfo?(dkeeler)
Flags: needinfo?(cviecco)
Flags: needinfo?(brian)
Duplicate of this bug: 1059262
So I think is the use of the scopedPLarena as a function variable, that could lead on the arena being freed earlier than expected. Brian, keeler? I moved it to a private variable so that the certs are ensured to live for the duration of the test.
Attached patch fix-use-after-free-gtests (obsolete) — Splinter Review
Attachment #8480031 - Flags: feedback?(brian)
Flags: needinfo?(cviecco)
Comment on attachment 8480031 [details] [diff] [review]
fix-use-after-free-gtests

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

this does not fix it.
Attachment #8480031 - Flags: feedback?(brian)
Attachment #8480031 - Attachment is obsolete: true
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #216)
> We were promised a fix nearly a month ago.

No, you weren't.

Anyway, like I said before, I'm working on changes that will indirectly fix this by changing how memory is managed in all the tests. if you need a fix sooner then I suggest somebody else work on that temporary fix.
Flags: needinfo?(brian)
Obviously you and I have differing definitions of "a few days" then. To avoid future misunderstandings, I'll take you up at your offer to just disable the test until it can be fixed whenever is convenient for you and we won't have to worry about it.
I was simple once I looked at the right place.

https://tbpl.mozilla.org/?tree=Try&rev=3c9e1a13ee42
Attachment #8481021 - Flags: review?(dkeeler)
Comment on attachment 8481021 [details] [diff] [review]
gtest-fix

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

Nice.
Attachment #8481021 - Flags: review?(dkeeler) → review+
Comment on attachment 8481021 [details] [diff] [review]
gtest-fix

Approval Request Comment
[Feature/regressing bug #]: 916629
[User impact if declined]: None
[Describe test coverage new/current, TBPL]: TBPL runs showing no ASAN gtest failures
[Risks and why]: none, this fixes an intermittent bug on debug ASAN builds 
[String/UUID change made/needed]: None
Attachment #8481021 - Flags: approval-mozilla-aurora?
Comment on attachment 8481021 [details] [diff] [review]
gtest-fix

Actually, this is a test-only fix, so it doesn't need approval. Sorry for that :(
Attachment #8481021 - Flags: approval-mozilla-aurora?
(In reply to David Keeler (:keeler) [use needinfo?] from comment #233)
> Nice.

Nice work. If this was really the fix, then the fix for bug 1059928 would fix it too.
bug 1059255 has turned into the funnel for this signature it seems.
https://hg.mozilla.org/mozilla-central/rev/d89d83ded337
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
(That last one looks like a misstar for what it is worth.)
You need to log in before you can comment on or make changes to this bug.