1905971 - process_reader is unable to parse large program header notes

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Gabriele Svelto [:gsvelto]]

This is an accidental regression from bug 1897189 which tightened the checks we do when parsing notes. Previously, if we encountered a note too large to be the one we were looking for, we'd just skip over it. Now we try to parse every note, so goblin is throwing failures when we try to parse a note that is larger than what we expect for the one we're looking for, as there's not enough bytes to parse the entire note. For example, in a debug AArch64/Android build we have these notes:

Displaying notes found in: .note.android.ident
  Owner                Data size 	Description
  Android              0x00000084	NT_VERSION (version)
   description data: 15 00 00 00 72 32 36 63 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 31 31 33 39 34 33 34 32 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Displaying notes found in: .note.moz.toolkit-build-id
  Owner                Data size 	Description
  mzbldid              0x0000000f	NT_VERSION (version)
   description data: 32 30 32 34 30 37 30 31 31 36 30 32 31 34 00

Displaying notes found in: .note.moz.annotation
  Owner                Data size 	Description
  mozannotation        0x00000010	Unknown note type: (0x415a4f4d)
   description data: b0 b3 90 0e 00 00 00 00 b0 fc ff ff ff ff ff ff

Displaying notes found in: .note.gnu.build-id
  Owner                Data size 	Description
  GNU                  0x00000010	NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: c7b2c7153640d25fc0da22c7e63b683f

When looking for .note.moz.annotation we'd read 44 bytes of data here which correspond to the expected size of the note we're looking for. However .note.android.ident is much larger, and this causes this to fail. Since we bail out upon hitting a failure, we never find the note we're looking for.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1897189

:gerard-majax, since you are the author of the regressor, bug 1897189, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Gabriele Svelto [:gsvelto]]

I've got a fix for this in the pipeline.

Comment 3

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dc1d5d07d6e
Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Can you please assign this a severity?

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8dc1d5d07d6e

Comment 7

[Donal Meehan [:dmeehan]]

:gsvelto do you want to add a beta and esr128 uplift request on this?
How about a release uplift for possible inclusion in a later Fx128 dot release?

Comment 8

[Gabriele Svelto [:gsvelto]]

Yes, it would be nice to uplift this as it fixes a rather important problem.

Comment 9

[Gabriele Svelto [:gsvelto]]

Comment on attachment 9410904 [details]
Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Beta/Release Uplift Approval Request

• User impact if declined: None, but crash reports on Android might be missing important information
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a small change to safe rust code that is covered by tests
• String changes made/needed: none
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: We would have limited visibility in Android crashes for the entire length of the ESR release
• User impact if declined: None, but crash reports on Android might be missing important information
• Fix Landed on Version: 130
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a small change to safe rust code that is covered by tests

Comment 10

[Donal Meehan [:dmeehan]]

Comment on attachment 9410904 [details]
Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

I just noticed this affecting Android only, rejecting esr128 uplift request since we don't ship Android builds of ESR.
This should still get a release uplift request though for Fx128

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9410904 [details]
Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Approved for 129.0b2

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/7807b93478af

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Per alissy, this could affect other Linux-based OSes too. Probably worth taking on ESR128 also to play it safe.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9410904 [details]
Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Approved for Fenix/Focus 128.0.1.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/f80f8c34f3dd

Comment 16

[Donal Meehan [:dmeehan]]

Comment on attachment 9410904 [details]
Bug 1905971 - Handle large PT_NOTE entries in the program headers correctly r=gerard-majax

Approved for 128.1esr.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/e73b7a8c1564