masaori335 commented on code in PR #13768:
URL: https://github.com/apache/trafficserver/pull/13768#discussion_r4204811349
##########
src/iocore/cache/CacheVC.cc:
##########
@@ -403,6 +403,19 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED
*/)
goto Ldone;
}
+ // Everything below trusts len and hlen, and STORE_COLLISION lets a doc
that is not ours get this far.
+ if (doc->magic == DOC_MAGIC &&
+ (doc->len < sizeof(Doc) || doc->len > io.aiocb.aio_nbytes || doc->hlen
> doc->len - sizeof(Doc))) {
+ // Callers check the key before trusting len, so a doc that is not ours
stays a collision and they keep probing.
+ // Marking it corrupt would fail a read whose own fragment may be
further down the chain.
+ if (doc->first_key == *read_key || doc->key == *read_key) {
+ Warning("Doc length %u (hlen %u) does not fit the %zu bytes read -
disk %s offset %" PRIu64, doc->len, doc->hlen,
+ static_cast<size_t>(io.aiocb.aio_nbytes),
stripe->hash_text.get(), static_cast<uint64_t>(stripe->vol_offset(&dir)));
+ doc->magic = DOC_CORRUPT;
+ }
Review Comment:
Fixed in f89d4e3824. openWriteOverwrite never checked DOC_CORRUPT, so this
was already possible for future-version and checksum-mismatch docs before this
PR. It now leaves first_buf empty for a corrupt doc, so the writer sees it the
same as a probe that found nothing. It still sets od->first_dir so the write
replaces the bad entry. openWriteStartDone already rejects magic != DOC_MAGIC.
I didn't make it an I/O error, because that would change every handleReadDone
caller, including the read paths that drop the bad dir and probe again. The new
test runs handleReadDone into openWriteOverwrite and checks the corrupt doc
stays out of first_buf.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
