Skip to content

Commit

Permalink
Bug 1832591: Add CRCs to preloader caches. r=mccr8
Browse files Browse the repository at this point in the history
We've been seeing crashes in the wild from multiple entries with empty hash
key names being read from the preloader caches. Based on assertions we've
added to the code, it appears that this is happening on disk, after the cache
has been written, rather than during runtime.

Using a CRC for this sort of data is a sensible precaution in any case, and it
should make it easier to distinguish between on-disk corruption that we can
safely ignore and other sorts of consistency issues that we should ideally
fix.

Differential Revision: https://phabricator.services.mozilla.com/D178118
  • Loading branch information
kmaglione committed May 16, 2023
1 parent 7c0aeaa commit ece8aee
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 4 deletions.
17 changes: 15 additions & 2 deletions js/xpconnect/loader/ScriptPreloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/Document.h"

#include "crc32c.h"
#include "js/CompileOptions.h" // JS::ReadOnlyCompileOptions
#include "js/experimental/JSStencil.h"
#include "js/Transcoding.h"
Expand Down Expand Up @@ -401,7 +402,7 @@ Result<nsCOMPtr<nsIFile>, nsresult> ScriptPreloader::GetCacheFile(
return std::move(cacheFile);
}

static const uint8_t MAGIC[] = "mozXDRcachev002";
static const uint8_t MAGIC[] = "mozXDRcachev003";

Result<Ok, nsresult> ScriptPreloader::OpenCache() {
MOZ_TRY(NS_GetSpecialDirectory("ProfLDS", getter_AddRefs(mProfD)));
Expand Down Expand Up @@ -510,7 +511,8 @@ Result<Ok, nsresult> ScriptPreloader::InitCacheInternal(
auto size = mCacheData->size();

uint32_t headerSize;
if (size < sizeof(MAGIC) + sizeof(headerSize)) {
uint32_t crc;
if (size < sizeof(MAGIC) + sizeof(headerSize) + sizeof(crc)) {
return Err(NS_ERROR_UNEXPECTED);
}

Expand All @@ -527,10 +529,17 @@ Result<Ok, nsresult> ScriptPreloader::InitCacheInternal(
headerSize = LittleEndian::readUint32(data.get());
data += sizeof(headerSize);

crc = LittleEndian::readUint32(data.get());
data += sizeof(crc);

if (data + headerSize > end) {
return Err(NS_ERROR_UNEXPECTED);
}

if (crc != ComputeCrc32c(~0, data.get(), headerSize)) {
return Err(NS_ERROR_UNEXPECTED);
}

{
auto cleanup = MakeScopeExit([&]() { mScripts.Clear(); });

Expand Down Expand Up @@ -726,8 +735,12 @@ Result<Ok, nsresult> ScriptPreloader::WriteCache() {
uint8_t headerSize[4];
LittleEndian::writeUint32(headerSize, buf.cursor());

uint8_t crc[4];
LittleEndian::writeUint32(crc, ComputeCrc32c(~0, buf.Get(), buf.cursor()));

MOZ_TRY(Write(fd, MAGIC, sizeof(MAGIC)));
MOZ_TRY(Write(fd, headerSize, sizeof(headerSize)));
MOZ_TRY(Write(fd, crc, sizeof(crc)));
MOZ_TRY(Write(fd, buf.Get(), buf.cursor()));

// Align the start of the scripts section to the transcode alignment.
Expand Down
17 changes: 15 additions & 2 deletions js/xpconnect/loader/URLPreloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "mozilla/Unused.h"
#include "mozilla/Vector.h"

#include "crc32c.h"
#include "MainThreadUtils.h"
#include "nsPrintfCString.h"
#include "nsDebug.h"
Expand Down Expand Up @@ -165,7 +166,7 @@ Result<nsCOMPtr<nsIFile>, nsresult> URLPreloader::GetCacheFile(
return std::move(cacheFile);
}

static const uint8_t URL_MAGIC[] = "mozURLcachev002";
static const uint8_t URL_MAGIC[] = "mozURLcachev003";

Result<nsCOMPtr<nsIFile>, nsresult> URLPreloader::FindCacheFile() {
nsCOMPtr<nsIFile> cacheFile;
Expand Down Expand Up @@ -235,8 +236,12 @@ Result<Ok, nsresult> URLPreloader::WriteCache() {
uint8_t headerSize[4];
LittleEndian::writeUint32(headerSize, buf.cursor());

uint8_t crc[4];
LittleEndian::writeUint32(crc, ComputeCrc32c(~0, buf.Get(), buf.cursor()));

MOZ_TRY(Write(fd, URL_MAGIC, sizeof(URL_MAGIC)));
MOZ_TRY(Write(fd, headerSize, sizeof(headerSize)));
MOZ_TRY(Write(fd, crc, sizeof(crc)));
MOZ_TRY(Write(fd, buf.Get(), buf.cursor()));
}

Expand All @@ -263,7 +268,8 @@ Result<Ok, nsresult> URLPreloader::ReadCache(
auto size = cache.size();

uint32_t headerSize;
if (size < sizeof(URL_MAGIC) + sizeof(headerSize)) {
uint32_t crc;
if (size < sizeof(URL_MAGIC) + sizeof(headerSize) + sizeof(crc)) {
return Err(NS_ERROR_UNEXPECTED);
}

Expand All @@ -278,10 +284,17 @@ Result<Ok, nsresult> URLPreloader::ReadCache(
headerSize = LittleEndian::readUint32(data.get());
data += sizeof(headerSize);

crc = LittleEndian::readUint32(data.get());
data += sizeof(crc);

if (data + headerSize > end) {
return Err(NS_ERROR_UNEXPECTED);
}

if (crc != ComputeCrc32c(~0, data.get(), headerSize)) {
return Err(NS_ERROR_UNEXPECTED);
}

{
mMonitor.AssertCurrentThreadOwns();

Expand Down
1 change: 1 addition & 0 deletions js/xpconnect/loader/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ LOCAL_INCLUDES += [
"/dom/base",
"/js/loader",
"/xpcom/base/",
"/xpcom/io", # crc32c.h
]

include("/ipc/chromium/chromium-config.mozbuild")

0 comments on commit ece8aee

Please sign in to comment.