paycheck: Check MOVE operations for start block 0.
paycheck now blocks payloads that read or write to block 0 in a MOVE
operation. It must do this because old versions of PReadAll and
PWriteAll could not seek to 0.
BUG=chromium:480751
TEST=unit tests, test_paycheck.sh
Change-Id: I042de444c92896496795af235e9386dca6c77be2
Reviewed-on: https://chromium-review.googlesource.com/267088
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Allie Wood <alliewood@chromium.org>
Trybot-Ready: Allie Wood <alliewood@chromium.org>
Tested-by: Allie Wood <alliewood@chromium.org>
diff --git a/host/lib/update_payload/checker.py b/host/lib/update_payload/checker.py
index 48b18e6..f2aa7bb 100644
--- a/host/lib/update_payload/checker.py
+++ b/host/lib/update_payload/checker.py
@@ -778,6 +778,14 @@
dst_idx = dst_extent.start_block
dst_num = dst_extent.num_blocks
+ # Check: start block is not 0. See crbug/480751; there are still versions
+ # of update_engine which fail when seeking to 0 in PReadAll and PWriteAll,
+ # so we need to fail payloads that try to MOVE to/from block 0.
+ if src_idx == 0 or dst_idx == 0:
+ raise error.PayloadError(
+ '%s: MOVE operation cannot have extent with start block 0' %
+ op_name)
+
if self.check_move_same_src_dst_block and src_idx == dst_idx:
raise error.PayloadError(
'%s: src/dst block number %d is the same (%d).' %
diff --git a/host/lib/update_payload/checker_unittest.py b/host/lib/update_payload/checker_unittest.py
index eb8b564..b57c2fc 100755
--- a/host/lib/update_payload/checker_unittest.py
+++ b/host/lib/update_payload/checker_unittest.py
@@ -633,7 +633,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 128)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 6)))
self.assertIsNone(
@@ -646,7 +646,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 128)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 6)))
self.assertRaises(
@@ -661,7 +661,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 127)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 127)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 6)))
self.assertRaises(
@@ -676,7 +676,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 128)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 5)))
self.assertRaises(
@@ -691,7 +691,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 128)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 5)))
self.assertRaises(
@@ -699,7 +699,7 @@
payload_checker._CheckMoveOperation,
op, None, 134, 134, 'foo')
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 129)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 129)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 6)))
self.assertRaises(
@@ -714,7 +714,7 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 4), (12, 2), (1024, 128)))
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((16, 128), (512, 7)))
self.assertRaises(
@@ -729,6 +729,21 @@
op.type = common.OpType.MOVE
self.AddToMessage(op.src_extents,
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
+ self.AddToMessage(op.dst_extents,
+ self.NewExtentList((8, 128), (512, 6)))
+ self.assertRaises(
+ update_payload.PayloadError,
+ payload_checker._CheckMoveOperation,
+ op, None, 134, 134, 'foo')
+
+ def testCheckMoveOperation_FailZeroStartBlock(self):
+ """Tests _CheckMoveOperation(); fails, has extent with start block 0."""
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ op = update_metadata_pb2.DeltaArchiveManifest.InstallOperation()
+ op.type = common.OpType.MOVE
+
+ self.AddToMessage(op.src_extents,
self.NewExtentList((0, 4), (12, 2), (1024, 128)))
self.AddToMessage(op.dst_extents,
self.NewExtentList((8, 128), (512, 6)))
@@ -737,6 +752,15 @@
payload_checker._CheckMoveOperation,
op, None, 134, 134, 'foo')
+ self.AddToMessage(op.src_extents,
+ self.NewExtentList((1, 4), (12, 2), (1024, 128)))
+ self.AddToMessage(op.dst_extents,
+ self.NewExtentList((0, 128), (512, 6)))
+ self.assertRaises(
+ update_payload.PayloadError,
+ payload_checker._CheckMoveOperation,
+ op, None, 134, 134, 'foo')
+
def testCheckBsdiff(self):
"""Tests _CheckMoveOperation()."""
payload_checker = checker.PayloadChecker(self.MockPayload())
@@ -829,10 +853,10 @@
common.OpType.SOURCE_COPY, common.OpType.SOURCE_BSDIFF):
if fail_src_extents:
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 0)))
+ self.NewExtentList((1, 0)))
else:
self.AddToMessage(op.src_extents,
- self.NewExtentList((0, 16)))
+ self.NewExtentList((1, 16)))
total_src_blocks = 16
if op_type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ):
