tree b4d04fdfc0ad50499246463f34630b290a1f3a77
parent a9649d19cae40eb74703888e8124780766a40470
author Mark Mentovai <mark@chromium.org> 1645812756 +0000
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> 1645812756 +0000

Revert "mac: Pad the unsigned official branded x86_64 main executable to ~257kB"

This reverts commit d70a026ed0c639e1089e04df0c75fec45a708037.

Reason for revert: Although I tested code-signing the output of this,
codesign --sign in production doesn’t like it. See
https://crbug.com/1300598#c8.

Original change's description:
> mac: Pad the unsigned official branded x86_64 main executable to ~257kB
>
> Seriously.
>
> When the main executable is updated on disk while the application is
> running, and the offset of the Mach-O image at the main executable’s
> path changes from the offset that was determined when the executable was
> loaded, SecCode ceases to be able to work with the executable. This may
> be triggered when the product is updated on disk but the application has
> not yet relaunched. This affects SecCodeCopySelf and
> SecCodeCopyGuestWithAttributes. Bugs are evident even when validation
> (SecCodeCheckValidity) is not attempted.
>
> Practically, this is only a concern for fat (universal) files, because
> the offset of a Mach-O image in a thin (single-architecture) file is
> always 0. The branded product always ships a fat executable, and because
> some uses of SecCode are in OS code beyond Chrome’s control, an effort
> is made to freeze the geometry of the branded (is_chrome_branded)
> for-public-release (is_official_build) main executable.
>
> The fat file is produced by installer/mac/universalizer.py. The x86_64
> slice always precedes the arm64 slice: lipo, as used by
> universalizer.py, always places the arm64 slice last. See Xcode 12.0
> https://github.com/apple-oss-distributions/cctools/blob/cctools-973.0.1/misc/lipo.c#L2672
> cmp_qsort, used by create_fat at #L962. universalizer.py ensures that
> the first slice in the file is located at a constant offset (16kB since
> 98.0.4758.80), but if the first slice’s size changes, it can affect the
> offset of the second slice, the arm64 one, triggering SecCode-related
> bugs for arm64 users across updates.
>
> As quite a hack of a workaround, the offset of the arm64 slice within
> the fat main executable is fixed at a constant value by introducing
> padding to the x86_64 slice that precedes it. The arm64 slice needs to
> remain at offset 304kB (since 98.0.4758.80), so enough padding is added
> to the x86_64 slice to ensure that the arm64 slice lands where it needs
> to be when universalized. This padding needs to be added to the thin
> form of the x86_64 image before being fed to universalizer.py.
>
> To make things extra tricky, the final size of the x86_64 image is not
> known when it is linked, because code signing will contribute more data
> to the file. Typically, official code signing adds a non-constant
> 22-23kB. The non-constancy makes a precise computation of the required
> padding at this stage of the build impossible. Luckily, the size of the
> x86_64 image doesn’t need to be so precise. The arm64 slice that follows
> it in the fat file will be 16kB-aligned, so it’s enough to ensure that
> the x86_64 slice ends at an offset in the range (288kB, 304kB], or,
> since the x86_64 slice itself begins at offset 16kB, its total size once
> signed must be in the range (272kB, 288kB]. Targeting size 280kB, right
> in the middle of that range, and assuming an expected 23200-byte
> contribution from code signing, the unsigned x86_64 image should be
> padded to 263520 bytes. Code signing may then add any amount in the
> range (15008 bytes, 31392 bytes] and the result, when universalized,
> will have its arm64 slice at the required 304kB offset.
>
> This introduces //build/toolchain/apple/pad_linkedit.py to insert the
> padding, which can be used by passing -Wcrl,pad_linkedit to
> linker_driver.py, and uses it when linking the x86_64 main executable in
> branded official builds. pad_linkedit.py can only increase the size of
> the image. It will raise an error on an attempt to decrease the size.
> Fortunately, the x86_64 main executable in this build configuration is
> currently smaller than this size, and is expected to shrink even further
> in the future.
>
> Bug: 1300598, 1295098
> Change-Id: Ib1b92c45ecae3d38752eb56103e436032e818c03
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3488454
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Mark Mentovai <mark@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#974951}

Bug: 1300598, 1295098
Change-Id: I7414faaf6c0e67379cb72aac31b92a12da9d7c06
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3489592
Auto-Submit: Mark Mentovai <mark@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975173}