Ticket #12055 (closed enhancement: wontfix)

Opened 2 years ago

Last modified 21 months ago

OFW should sanity-check size of .zd files before starting to write them

Reported by: cjb Owned by: Quozl
Priority: normal Milestone: 13.1.0
Component: build-system Version: not specified
Keywords: Cc: wmb@…, martin.langhoff, dsd
Action Needed: no action Verified: no
Deployments affected: Blocked By:
Blocking:

Description

The failure mode I'm trying to protect against is the one where you only have a partial download of the file, and you only realize it after fs-update spends five minutes flashing and then says "Short read of zdata file" unexpectedly. It would save time (and wondering about why it failed) for it to immediately tell you that there's no way the .zd file you've provided can be complete, instead of waiting until it runs out of data.

We should store the ultimate file size in bytes in a header of the .zd file, and test it against the file size on disk before doing anything else.

Attachments

0001-zhashfs-add-header-data-for-sanity-check-of-file-siz.patch (1.7 kB) - added by Quozl 2 years ago.
olpc-os-builder patch for review

Change History

Changed 2 years ago by Quozl

olpc-os-builder patch for review

Changed 2 years ago by Quozl

  • cc wmb@…, martin.langhoff, dsd added; wmb removed
  • status changed from new to assigned
  • next_action changed from never set to review
  • component changed from ofw - open firmware to build-system

Martin, Daniel, olpc-os-builder support for sanity-check of file size requires a patch, see previous change to this ticket. Please review and commit. Let me know when it is in the XO-4 builds.

Open Firmware support added in svn 3204. Will be included in next release.

(Some bugs or filesystem corruptions have a footprint of filesize not matching amount of data read, so solving this ticket will also detect these.)

Changed 2 years ago by Quozl

Released in Q3C09 for XO-1.5. Patch to olpc-os-builder is still not merged.

Changed 2 years ago by dsd

  • next_action changed from review to add to build
  • milestone changed from Not Triaged to 13.1.0

Thanks, pushed to olpc-os-builder master after a quick test.

Changed 2 years ago by dsd

  • next_action changed from add to build to test in build

Test in 13.1.0 build 1.

Changed 2 years ago by greenfeld

  • status changed from assigned to closed
  • next_action changed from test in build to no action
  • resolution set to fixed

Installed 13.1.0 os1 using Q3C07 to verify new .zd could be installed by older OFW. After reflashing verified that Q3C09 refused to install a file which was just the first 5 MB of 13.1.0 os1.

The new firmware was used to install 11.3.1 os885 as well as 13.1.0 os1 again to verify compatibility with older .zd files.

Tested on XO-1.5; to the best of my knowledge this is not in Q4D21 for XO-1.75 as the latter will try to install truncated files.

Changed 23 months ago by dsd

  • status changed from closed to reopened
  • resolution deleted

This change has broken the secure fs-update path from Q3C08 or older. When trying a secure fs-update to install a .zd file including the size line, this error occurs before image flashing starts:

Trying disk:\fs1.zip
Filesystem image found -
<buffer@1c0000>:9: Spec line mismatch

Changed 23 months ago by Quozl

  • status changed from reopened to closed
  • resolution set to duplicate

Being handled in #12186.

Changed 23 months ago by dsd

  • status changed from closed to reopened
  • resolution deleted

I would imagine that #12186 is a different issue: writing to data-buffer 0 is bad, but hasn't caused known failures (OFW was always doing it before r3369). And Q3C09 was already working before the #12186 fix. What needs to be addressed now is compatibility of the size: feature in secure image installs for versions such as Q3C07 and Q3C08 which are deployed in the field.

Changed 23 months ago by Quozl

Thanks. Can you provide signed builds to test against please?

Changed 23 months ago by Quozl

Never mind, saw the test case instructions in #12186.

Changed 23 months ago by Quozl

  • next_action changed from no action to code

Reproduced with 31005o4.zd with bundle-suffix set to 4. data-buffer is zero, but as you say harmlessly so. ?compare-spec-line called by zblocks: is failing with "Spec line mismatch" because the two lines are different; one is zblocks: 20000 3bcf, the other is [ifdef] size: size: 00003052d19d [then]. I see no obvious solution, so I recommend you revert the olpc-os-builder patch, and then we can close this ticket as wontfix.

Changed 23 months ago by dsd

  • status changed from reopened to closed
  • resolution set to wontfix

OK, done. Thanks for investigating. That will take effect for 13.1.0 build 8 and onwards.

Changed 21 months ago by Quozl

  • next_action changed from code to no action

Is in Q4D25 targeted for 13.2.0.

Note: See TracTickets for help on using tickets.