Closed
Bug 1003208
Opened 10 years ago
Closed 10 years ago
HTTP cache v2: CacheFileHandle::mHash is accessed after the memory is freed
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: michal, Assigned: michal)
Details
Attachments
(1 file)
6.04 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
We close all handles during shutdown in CacheFileIOManager::ShutdownInternal() and after this point all hashes are freed and no handle that lives longer must dereference its mHash member, since it points to a freed memory. The hash was dereferenced by CacheFileHandle::Log() called from CacheFileIOManager::CloseHandleInternal(). We must not call CacheFileIOManager::CloseHandleInternal() from the destructor if the handle is already closed. The patch fixes this and also nulls out mHash member so that the code crashes if there is yet another use of the freed memory. https://tbpl.mozilla.org/?tree=Try&rev=650179bf6ebc
Attachment #8414516 -
Flags: review?(honzab.moz)
Comment 1•10 years ago
|
||
Comment on attachment 8414516 [details] [diff] [review] patch v1 Review of attachment 8414516 [details] [diff] [review]: ----------------------------------------------------------------- Hmm.. kinda dirty fix. Can you please file a bug for clean up and make it block cache2afterenable?
Attachment #8414516 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Comment 2•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0941e6502ee8
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #1) > Hmm.. kinda dirty fix. Can you please file a bug for clean up and make it > block cache2afterenable? What exactly is dirty according to you and should be cleaned up?
Comment 4•10 years ago
|
||
(In reply to Michal Novotny (:michal) from comment #3) > (In reply to Honza Bambas (:mayhemer) from comment #1) > > Hmm.. kinda dirty fix. Can you please file a bug for clean up and make it > > block cache2afterenable? > > What exactly is dirty according to you and should be cleaned up? "Null out the pointer so that we crash if there is a bug in this code" - I think we should fix the code so that it can never crash. This is low priority, but when we change something around here I think it can occasionally start null-crashing or be somehow racy and do bad memory access again.
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0941e6502ee8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•