Closed
Bug 1038554
Opened 9 years ago
Closed 9 years ago
CacheIndex::ParseRecords() a possible bug
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: dragana, Assigned: michal)
Details
Attachments
(1 file)
2.20 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
2108 if (pos != mRWBufPos) { 2109 memmove(mRWBuf, mRWBuf + pos, mRWBufPos - pos); 2110 mRWBufPos -= pos; 2111 pos = 0; 2112 } 2113 2114 int64_t fileOffset = sizeof(CacheIndexHeader) + 2115 mSkipEntries * sizeof(CacheIndexRecord) + mRWBufPos; I think it is a bug. If pos == mRWBufPos the fileoffset will be 2 times what it should be. can we guaranty that they are always different? the size of pos will be sizeof(CacheIndexHeader) + mSkipEntries * sizeof(CacheIndexRecord) before this lines.
Assignee | ||
Comment 1•9 years ago
|
||
Yes, this is a bug. But luckily it is never triggered because of buffer size and sizeof(CacheIndexHeader). When we have all remaining data in the buffer there is always a hash at the end, so pos is mRWBufPos - 4. When this is a beginning of the file, we parse the header, 454 entries and 28 bytes is left unparsed. All other ParseRecords() iterations parse 455 entries and 4 bytes is always left unparsed.
Assignee: nobody → michal.novotny
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8542564 -
Flags: review?(honzab.moz)
![]() |
||
Comment 3•9 years ago
|
||
Comment on attachment 8542564 [details] [diff] [review] fix Review of attachment 8542564 [details] [diff] [review]: ----------------------------------------------------------------- I can only trust you on this. All the ptr arithmetic in this code should one nice day [ :)) ] go to a nice class so that we won't need to worry...
Attachment #8542564 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ba79118e686
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2ba79118e686
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in
before you can comment on or make changes to this bug.
Description
•