Closed Bug 1732885 Opened 3 years ago Closed 1 year ago

Investigate bug 1732249

Categories

(Core :: Networking: HTTP, defect, P2)

defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr102 108+ fixed

People

(Reporter: dragana, Unassigned)

References

Details

(Keywords: crash, csectype-wildptr, sec-moderate, Whiteboard: [maybe fixed by 1797370][necko-triaged])

Crash Data

Attachments

(1 obsolete file)

We will add some diagnostic asserts to figure out why certificates are not already set when response is received.

Pushed by ddamjanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5bb05a86bd2e
Add assertions to make sure certificates are set if a handshake succeeds. r=necko-reviewers,valentin
Regressions: 1733279
Regressions: 1733287
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

Backed out changeset 5bb05a86bd2e (Bug 1732885) for causing networking crashes. a=backout
Backout link: https://hg.mozilla.org/mozilla-central/rev/75ff6f392acad0ccffff8a6484dfcb0bfd1bc408
Autoland backout link: https://hg.mozilla.org/mozilla-central/rev/75ff6f392acad0ccffff8a6484dfcb0bfd1bc408

Status: RESOLVED → REOPENED
Flags: needinfo?(dd.mozilla)
Resolution: FIXED → ---
Target Milestone: 94 Branch → ---
Crash Signature: [@ mozilla::net::nsHttpTransaction::CheckCert] [@ mozilla::net::nsHttpTransaction::ProcessData]
Crash Signature: [@ mozilla::net::nsHttpTransaction::CheckCert] [@ mozilla::net::nsHttpTransaction::ProcessData] → [@ mozilla::net::nsHttpTransaction::CheckCert] [@ mozilla::net::nsHttpTransaction::ProcessData] [@ mozilla::net::nsHttpConnection::OnReadSegment]

I hit this yesterday on my Android nightly, too and thus took a look. At SetServerCert (which seems to be the only place we set mServerCert) there is a possible and probably expected code path that leaves us with a nullptr inside mServerCert if we have an mListener set. That means probably that we cannot expect an existing mSecurityInfo to always carry an initialized certificate?

(In reply to Jens Stutte [:jstutte] from comment #6)

I hit this yesterday on my Android nightly, too and thus took a look. At SetServerCert (which seems to be the only place we set mServerCert) there is a possible and probably expected code path that leaves us with a nullptr inside mServerCert if we have an mListener set. That means probably that we cannot expect an existing mSecurityInfo to always carry an initialized certificate?

This is a server side socket, httpTransactions never hit that code.

Flags: needinfo?(dd.mozilla)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:dragana, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(dd.mozilla)
Flags: needinfo?(valentin.gosu)
Severity: -- → N/A

The patch was made to investigate an issue, it will not land permanently.

Flags: needinfo?(dd.mozilla)
Attachment #9243312 - Attachment is obsolete: true

I will not have time to work on this.

Assignee: dd.mozilla → nobody

often-wildptr crashes; ~3-4 per week.

No CheckCert() failures in the last 6 months, so the diagnostic appears to be showing us nothing.

A fair number of the wildptr crashes seem to be at the call to HandleContent()

Group: core-security
Priority: P2 → --
Type: task → defect
Group: core-security → network-core-security
Severity: N/A → S3
Priority: -- → P2

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:valentin, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Flags: needinfo?(valentin.gosu)

It's stalled.

Severity: S3 → S2
Flags: needinfo?(valentin.gosu)

I don't see any crashes in recent builds. a bunch of 102.5 which was the current ESR until last week, but nothing on main newer that 105

Maybe this is WFM now?

Flags: needinfo?(valentin.gosu)
Keywords: sec-highsec-moderate

If I were to guess I think it was bug 1797370 that fixed this. Thank you for the regression range Ryan!

Flags: needinfo?(valentin.gosu)

Still no more crashes. Closing as WFM.

Status: REOPENED → RESOLVED
Closed: 3 years ago1 year ago
Resolution: --- → WORKSFORME

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit auto_nag documentation.

Keywords: stalled
See Also: → 1797370
Whiteboard: [necko-triaged] → [maybe fixed by 1797370][necko-triaged]
Group: network-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: