Closed Bug 531425 Opened 11 years ago Closed 11 years ago

Firefox 3.6b4 Crash [@ nsAuthSSPI::GetNextToken(void const*, unsigned int, void**, unsigned int*) ]

Categories

(Core :: Networking: HTTP, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta5-fixed

People

(Reporter: chofmann, Assigned: jimm)

References

Details

Attachments

(1 file, 2 obsolete files)

there is a new crash that shows up in beta4, not previously seen.  could be a regression or could be a morf of the signature name.

http://people.mozilla.com/~chofmann/crash-data/new-crashes/new-crashes-36b4-from-36b1-4-35all.html

signature looks like

https://crash-stats.mozilla.com/report/index/70d5e3de-8847-4f74-a423-458152091127

Frame  	Module  	Signature [Expand]  	Source
0 	xul.dll 	nsAuthSSPI::GetNextToken 	extensions/auth/nsAuthSSPI.cpp:372
1 	xul.dll 	nsHttpAuthCache::LookupAuthNode 	netwerk/protocol/http/src/nsHttpAuthCache.cpp:222


more reports at 

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=nsAuthSSPI::GetNextToken%28void%20const*,%20unsigned%20int,%20void**,%20unsigned%20int*%29&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsAuthSSPI::GetNextToken%28void%20const*,%20unsigned%20int,%20void**,%20unsigned%20int*%29&page=1

changes around the source file at the top of the stack indicates possible regression to investigate

396a38c7a6ae
2009-11-19 16:18 -0600	Jim Mathies - Bug 520607 - Remove use of "ntlm" auth module and replace with use of "sys-ntlm". r=wtc, cbiesinger. sr=bz.
OS: Mac OS X → Windows XP
Flags: blocking1.9.2?
Comments might suggest a work around or maybe STR.  dirty profile?

> I installed 3.6b4 alongside 3.5.5, in /opt on SuSE 11.2 This crash occurs when I run /opt/firefox/firefox as a regular user. It doesn't happen if I run as root. Restarting also fails.

> This happens when I start FF 3.6b4 with my exisiting ~/.mozilla directory in place. It is OK if I remove that directory
(In reply to comment #1)
> Comments might suggest a work around or maybe STR.  dirty profile?
> 
> > I installed 3.6b4 alongside 3.5.5, in /opt on SuSE 11.2 This crash occurs when I run /opt/firefox/firefox as a regular user. It doesn't happen if I run as root. Restarting also fails.
> 
> > This happens when I start FF 3.6b4 with my exisiting ~/.mozilla directory in place. It is OK if I remove that directory

Chris, these comment don't seem to match to this crash?
Assignee: nobody → jmathies
yeah,  sorry for the confusion.  comment 1 belongs in Bug 531305
Flags: blocking1.9.2? → blocking1.9.2+
Attached patch cache creds patch.v1 (obsolete) — Splinter Review
Best guess - these credential strings need to be valid for the duration of the connection. The auth cache pointers may not stay valid, so we keep a local copy in the auth module.
Attachment #415462 - Flags: review?(bzbarsky)
Can't you just use an nsString for the password too?  If you feel strongly about the SecureZeroMemory thing, you could do that using BeginWriting on the string, no?
(In reply to comment #5)
> Can't you just use an nsString for the password too?  If you feel strongly
> about the SecureZeroMemory thing, you could do that using BeginWriting on the
> string, no?

I guess I'm being too paranoid. I'd much rather just use an nsString. Another patch coming up.
Attached patch cache creds patch.v2 (obsolete) — Splinter Review
updated to comments.
Attachment #415462 - Attachment is obsolete: true
Attachment #415485 - Flags: review?(bzbarsky)
Attachment #415462 - Flags: review?(bzbarsky)
Comment on attachment 415485 [details] [diff] [review]
cache creds patch.v2

Make those reinterpret_cast, and looks good.
Attachment #415485 - Flags: review?(bzbarsky) → review+
(In reply to comment #8)
> (From update of attachment 415485 [details] [diff] [review])
> Make those reinterpret_cast, and looks good.

I need the const_cast to ditch the const on nsAString's internal char_type.
In that case, do a separate reinterpret_cast to const unsigned short* and then a const_cast to the non-const type?

And are we sure the callee won't modify this no-longer-const buffer?
(In reply to comment #10)
> In that case, do a separate reinterpret_cast to const unsigned short* and then
> a const_cast to the non-const type?
> 
> And are we sure the callee won't modify this no-longer-const buffer?

I can't see any reason why tit would. The api docs don't indicate any change will occur.
OK.  We're not sharing these strings with anyone anyway, so that should be ok.
The tree is looking pretty busted at the moment, will check this in in the am.
So why not use BeginWriting() instead of get()+const_cast?
(In reply to comment #14)
> So why not use BeginWriting() instead of get()+const_cast?

Looks like BeginWriting returns the same type - char_type*.
Correct. It avoids the const_cast.
Yeah, I'd prefer the BeginWriting() too.  That would make it clear that we're not ensuring immutability anymore.
Updated, kind of strange using a method named "BeginWriting" when I'm not writing anything, but such is life I guess with our string classes.
Attachment #415485 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/e58a42937197
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [needs 1.9.2 landing]
Blocks: 520607
You need to log in before you can comment on or make changes to this bug.