1038554 - CacheIndex::ParseRecords() a possible bug

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Dragana Damjanovic [:dragana]]

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.

Comment 1

[Michal Novotny [:michal]]

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.

Comment 2

[Michal Novotny [:michal]]

Attachment: fix

Comment 3

[Honza Bambas (:mayhemer)]

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...

Comment 4

[Michal Novotny [:michal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ba79118e686

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2ba79118e686