1548356 - Hash the XDR transcode buffer

Status: NEW

(Core :: JavaScript Engine, enhancement, P2)

Description

[Ted Campbell [:tcampbell]]

Since the XDR data integrity of startup caches is important to avoid crashes, we should add a hash check on decode.

One concern would be disk read, but the hash is per transcode operation which are done as a chunk and would quickly hit the same disk latency anyways. The startup caches perform several transcode operations so the hash does not span the whole file. The worst case I see is the activity-stream bundle which is a 750kB buffer.

This also lumps the first-access of the data in one place which might be useful if we wanted to have a signal handling to catch mmap errors. See Bug 1444442.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1548356 - Remove unused XDRBuffer::uptr

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1548356 - Remove XDRState::codeDouble

Depends on D29551

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1548356 - Check SHA1 signature of XDR data

Compute and verify the SHA1 hash of each XDR transcode operation. This
is to detect data corruption issues that will otherwise lead to serious
crashes.

Depends on D29552

Comment 4

[Nicolas B. Pierron [:nbp]]

Stupid question, any reason to use SHA1, and not a cheaper / faster-to-compute hashing / check-sum algorithm?

Comment 5

[Ted Campbell [:tcampbell]]

The silly reason was that it was available in MFBT. There are several CRC implementations in tree but not in JS so SHA1 was the easiest to get. Several of the CRC implementations are not very efficient either. A number of those CRC implementations are byte-based and naive as well. It is probably worth replacing the SHA1 before landing so no reviewers have been assigned yet.